[Nouveau] Kernel patch: validate nouveau_channel_get id argument

[Michel Hermier]

Feel free to correct the patch as you need/want, It is so simple and
tricky, that I don't really care my rights for a 2 liner patch ;) If
you really insist, I'll prepare the patch following the guide lines.

2010/12/25 Francisco Jerez <currojerez at riseup.net>:
> Michel Hermier <michel.hermier at gmail.com> writes:
>
>> Hi,
>> While hacking libdrm I triggered a kernel oups due to a non checked
>> argument from user land.
>> In nouveau_ioctl_notifier_alloc, nouveau_channel_get is invoked, but
>> it doesn't validate the na->channel input argument. The attached patch
>> validates the channel index, and change it's type to uint32_t since it
>> is an index after all.
>>
> Thank you, some minor comments inline.
>
>> Cheers,
>>     Michel
>>
>> From dc00e5ccce3f10e51ae143d6dda6aa8febab271d Mon Sep 17 00:00:00 2001
>> From: Michel Hermier <hermier at frugalware.org>
>> Date: Fri, 24 Dec 2010 14:49:13 +0100
>> Subject: [PATCH] Fix channel nouveau_channel_get index type and check it's value.
>
> We usually prefix our kernel commit messages with "drm/nouveau: " or
> something similar, to tell them apart from the huge kernel commit
> flow. Also you made a small typo in "it's".
>
>>
> "Signed-off-by" line missing. You should have a look at
> "Documentation/SubmittingPatches" and "Documentation/CodingStyle", if
> you haven't already.
>
>> ---
>>  drivers/gpu/drm/nouveau/nouveau_channel.c |    5 ++++-
>>  drivers/gpu/drm/nouveau/nouveau_drv.h     |    2 +-
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_channel.c b/drivers/gpu/drm/nouveau/nouveau_channel.c
>> index e37977d..bc07a61 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_channel.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_channel.c
>> @@ -247,12 +247,15 @@ nouveau_channel_get_unlocked(struct nouveau_channel *ref)
>>  }
>>
>>  struct nouveau_channel *
>> -nouveau_channel_get(struct drm_device *dev, struct drm_file *file_priv, int id)
>> +nouveau_channel_get(struct drm_device *dev, struct drm_file *file_priv, uint32_t id)
> This goes above the 80 column limit. Anyway I'd leave this line alone,
> we're already using ints as channel indices in most places.
>
>>  {
>>       struct drm_nouveau_private *dev_priv = dev->dev_private;
>>       struct nouveau_channel *chan;
>>       unsigned long flags;
>>
>> +     if (unlikely(id >= NOUVEAU_MAX_CHANNEL_NR))
>> +             return ERR_PTR(-EINVAL);
>> +
>>       spin_lock_irqsave(&dev_priv->channels.lock, flags);
>>       chan = nouveau_channel_get_unlocked(dev_priv->channels.ptr[id]);
>>       spin_unlock_irqrestore(&dev_priv->channels.lock, flags);
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
>> index e815756..ec3eed2 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_drv.h
>> +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
>> @@ -870,7 +870,7 @@ extern int  nouveau_channel_alloc(struct drm_device *dev,
>>  extern struct nouveau_channel *
>>  nouveau_channel_get_unlocked(struct nouveau_channel *);
>>  extern struct nouveau_channel *
>> -nouveau_channel_get(struct drm_device *, struct drm_file *, int id);
>> +nouveau_channel_get(struct drm_device *, struct drm_file *, uint32_t id);
>>  extern void nouveau_channel_put_unlocked(struct nouveau_channel **);
>>  extern void nouveau_channel_put(struct nouveau_channel **);
>>  extern void nouveau_channel_ref(struct nouveau_channel *chan,
>> --
>> 1.7.3.4
>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nouveau
>