Nouveau - Dec 2010 - Kernel patch: validate nouveau_channel_get id argument

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.

Cheers,
    Michel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-channel-nouveau_channel_get-index-type-and-check.patch
Type: application/octet-stream
Size: 2041 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/nouveau/attachments/20101224/cd351770/attachment.obj>

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/nouveau/attachments/20101225/f4abcc80/attachment.pgp>