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

On Wed, Mar 12, 2014 at 4:45 PM, Emil Velikov emil.l.velikov@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@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@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel