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 <[email protected]>: > Michel Hermier <[email protected]> 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 <[email protected]> >> 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 >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/nouveau > _______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
