[PATCH 2/3] nouveau: sanitise NOUVEAU_LIBDRM_*_LIMIT_PERCENT input

[Ilia Mirkin]

On Wed, Mar 12, 2014 at 4:45 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> Current handling relies on atoi which does not detect errors
> additionally, any integer value will be considered as a valid
> percent.
>
> Resolve that by using strtol and limiting the value within
> the 5-100 (percent) range.
>
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>  nouveau/nouveau.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c
> index d6013db..06cd804 100644
> --- a/nouveau/nouveau.c
> +++ b/nouveau/nouveau.c
> @@ -76,7 +76,7 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev)
>         struct nouveau_device *dev = &nvdev->base;
>         uint64_t chipset, vram, gart, bousage;
>         drmVersionPtr ver;
> -       int ret;
> +       int ret, limit;
>         char *tmp;
>
>  #ifdef DEBUG
> @@ -121,16 +121,22 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev)
>
>         nvdev->close = close;
>
> +       nvdev->vram_limit_percent = 80;
>         tmp = getenv("NOUVEAU_LIBDRM_VRAM_LIMIT_PERCENT");
> -       if (tmp)
> -               nvdev->vram_limit_percent = atoi(tmp);
> -       else
> -               nvdev->vram_limit_percent = 80;
> +       if (tmp) {
> +               limit = strtol(tmp, NULL, 10);
> +               if (limit >= 5 && limit <= 100)
> +                       nvdev->vram_limit_percent = limit;
> +       }

Wouldn't it be easier to just clamp the value no matter what? i.e.
leave the current code alone, and add a clamp helper function + use
it? These are pretty rare env vars to set... presumably the person
setting them knows what they're doing. And if not, they get what they
deserve.

On a related topic, I'd personally rather not throw in arbitrary
restrictions to code like this -- it makes debugging harder later on.
(e.g. what's wrong with 0 percent? let's say i want to disable
gart/vram entirely?)

> +
> +       nvdev->gart_limit_percent = 80;
>         tmp = getenv("NOUVEAU_LIBDRM_GART_LIMIT_PERCENT");
> -       if (tmp)
> -               nvdev->gart_limit_percent = atoi(tmp);
> -       else
> -               nvdev->gart_limit_percent = 80;
> +       if (tmp) {
> +               limit = strtol(tmp, NULL, 10);
> +               if (limit >= 5 && limit <= 100)
> +                       nvdev->gart_limit_percent = limit;
> +       }
> +
>         DRMINITLISTHEAD(&nvdev->bo_list);
>         nvdev->base.object.oclass = NOUVEAU_DEVICE_CLASS;
>         nvdev->base.lib_version = 0x01000000;
> --
> 1.9.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel