Fixing arbitrary kernel leak in case FBIOGETCMAP_SPARC in sbusfb_ioctl_helper().
PATCH 1/1 Fixing arbitrary kernel leak in case FBIOGETCMAP_SPARC in sbusfb_ioctl_helper().
--- drivers/video/fbdev/sbuslib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/video/fbdev/sbuslib.c +2 -2
@@ -122,7 +122,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ured;unsigned char __user *ugreen;unsigned char __user *ublue; -int index, count, i; +unsigned int index, count, i; if (get_user(index, &c->index) || __get_user(count, &c->count) ||
@@ -161,7 +161,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ugreen;unsigned char __user *ublue;struct fb_cmap *cmap = &info->cmap; -int index, count, i; +unsigned int index, count, i;u8 red, green, blue; if (get_user(index, &c->index) ||
Signature · 1 line
2.14.3
Hi Peter,
Quoted 1 line · Peter Malone
Signed-off-by Peter Malone <[email protected]>
AFAIK empty commit message is not acceptable upstream
Quoted 32 lines
drivers/video/fbdev/sbuslib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/video/fbdev/sbuslib.c +2
-3@@ -122,7 +122,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ured;unsigned char __user *ugreen;unsigned char __user *ublue; -int index, count, i; +unsigned int index, count, i; if (get_user(index, &c->index) || __get_user(count, &c->count) ||@@ -161,7 +161,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ugreen;unsigned char __user *ublue;struct fb_cmap *cmap = &info->cmap; -int index, count, i; +unsigned int index, count, i;u8 red, green, blue; if (get_user(index, &c->index) || --2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 1/1 Fixing arbitrary kernel leak in case FBIOGETCMAP_SPARC in sbusfb_ioctl_helper().
Fixing arbitrary kernel leak in case FBIOGETCMAP_SPARC in sbusfb_ioctl_helper().
'index' is defined as an int in sbusfb_ioctl_helper(). We retrieve this from the user:
if (get_user(index, &c->index) ||
__get_user(count, &c->count) ||
__get_user(ured, &c->red) ||
__get_user(ugreen, &c->green) ||
__get_user(ublue, &c->blue))
return -EFAULT;and then we use 'index' in the following way:
red = cmap->red[index + i] >> 8;
green = cmap->green[index + i] >> 8;
blue = cmap->blue[index + i] >> 8;
This is a classic information leak vulnerability. 'index' should be an unsigned int, given its usage above.
This patch is straight-forward; it changes 'index' to unsigned int in two switch-cases: FBIOGETCMAP_SPARC && FBIOPUTCMAP_SPARC.
---
v2: fixed formatting
drivers/video/fbdev/sbuslib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/video/fbdev/sbuslib.c +2 -2
@@ -122,7 +122,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ured;unsigned char __user *ugreen;unsigned char __user *ublue; -int index, count, i; +unsigned int index, count, i; if (get_user(index, &c->index) || __get_user(count, &c->count) ||
@@ -161,7 +161,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ugreen;unsigned char __user *ublue;struct fb_cmap *cmap = &info->cmap; -int index, count, i; +unsigned int index, count, i;u8 red, green, blue; if (get_user(index, &c->index) ||
Signature · 1 line
2.14.3
Hi Peter,
Quoted 17 lines · Peter Malone
Fixing arbitrary kernel leak in case
FBIOGETCMAP_SPARCinsbusfb_ioctl_helper().'index' is defined as an int in
sbusfb_ioctl_helper(). We retrieve this from the user:
if (get_user(index, &c->index)||__get_user(count, &c->count) || __get_user(ured, &c->red) || __get_user(ugreen, &c->green) || __get_user(ublue, &c->blue)) return -EFAULT;and then we use 'index' in the following way:
red = cmap->red[index + i] >> 8;
green = cmap->green[index + i] >> 8;
blue = cmap->blue[index + i] >> 8;
Quoted 8 lines
This is a classic information leak vulnerability. 'index' should be an unsigned int, given its usage above.
This patch is straight-forward; it changes 'index' to unsigned int in two switch-cases:
FBIOGETCMAP_SPARC&&FBIOPUTCMAP_SPARC.Signed-off-by Peter Malone <[email protected]>---
much better :)
Quoted 28 lines
v2: fixed formatting
drivers/video/fbdev/sbuslib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/video/fbdev/sbuslib.c +2
-3@@ -122,7 +122,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ured;unsigned char __user *ugreen;unsigned char __user *ublue; -int index, count, i; +unsigned int index, count, i; if (get_user(index, &c->index) || __get_user(count, &c->count) ||@@ -161,7 +161,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ugreen;unsigned char __user *ublue;struct fb_cmap *cmap = &info->cmap; -int index, count, i; +unsigned int index, count, i;u8 red, green, blue; if (get_user(index, &c->index) || --
Quoted 2 lines
2.14.3
By just looking at the code and commit message:
Hi folks,
CVE-2018-6412 has been created for this. Is it possible for you to add a note indicating the CVE number when merging the patch?
I received the CVE number after the patch was created and ack'd, which is why I didn't include it in the commit message.
Quoted 61 lines · Mathieu Malaterre
Hi Peter,
Quoted 17 lines · Peter Malone
Fixing arbitrary kernel leak in case
FBIOGETCMAP_SPARCinsbusfb_ioctl_helper().'index' is defined as an int in
sbusfb_ioctl_helper(). We retrieve this from the user:
if (get_user(index, &c->index)||__get_user(count, &c->count) || __get_user(ured, &c->red) || __get_user(ugreen, &c->green) || __get_user(ublue, &c->blue)) return -EFAULT;and then we use 'index' in the following way:
red = cmap->red[index + i] >> 8;
green = cmap->green[index + i] >> 8;
blue = cmap->blue[index + i] >> 8;Quoted 8 lines
This is a classic information leak vulnerability. 'index' should be an unsigned int, given its usage above.
This patch is straight-forward; it changes 'index' to unsigned int in two switch-cases:
FBIOGETCMAP_SPARC&&FBIOPUTCMAP_SPARC.Signed-off-by Peter Malone <[email protected]>---
much better :)
Review
Quoted 6 lines
v2: fixed formatting
drivers/video/fbdev/sbuslib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)(quoted diff) +0
-0
b/drivers/video/fbdev/sbuslib.cdrivers/video/fbdev/sbuslib.c int sbusfb_ioctl_helper(unsigned long cmd, unsigned
Quoted 4 lines
index
af6fc97f4ba4..a436d44f1b7f100644drivers/video/fbdev/sbuslib.c +0
-0@@ -122,7 +122,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsignedlong arg,
drivers/video/fbdev/sbuslib.c int sbusfb_ioctl_helper(unsigned long cmd, unsigned
Quoted 9 lines
drivers/video/fbdev/sbuslib.c +1
-1unsigned char __user *ured;unsigned char __user *ugreen;unsigned char __user *ublue; -int index, count, i; +unsigned int index, count, i; if (get_user(index, &c->index) || __get_user(count, &c->count) ||@@ -161,7 +161,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsignedlong arg,
Quoted 7 lines
drivers/video/fbdev/sbuslib.c +1
-1unsigned char __user *ugreen;unsigned char __user *ublue;struct fb_cmap *cmap = &info->cmap; -int index, count, i; +unsigned int index, count, i;u8 red, green, blue;
Quoted 9 lines
Quoted 4 lines
drivers/video/fbdev/sbuslib.c +0
-1if (get_user(index, &c->index) || --2.14.3
By just looking at the code and commit message:
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi folks,
CVE-2018-6412 has been created for this. Is it possible for you to add a note indicating the CVE number when merging the patch?
I received the CVE number after the patch was created and ack'd, which is why I didn't include it in the commit message.
Quoted 65 lines · Mathieu Malaterre
Hi Peter,
Quoted 17 lines · Peter Malone
Fixing arbitrary kernel leak in case
FBIOGETCMAP_SPARCinsbusfb_ioctl_helper().'index' is defined as an int in
sbusfb_ioctl_helper(). We retrieve this from the user:
if (get_user(index, &c->index)||__get_user(count, &c->count) || __get_user(ured, &c->red) || __get_user(ugreen, &c->green) || __get_user(ublue, &c->blue)) return -EFAULT;and then we use 'index' in the following way:
red = cmap->red[index + i] >> 8;
green = cmap->green[index + i] >> 8;
blue = cmap->blue[index + i] >> 8;Quoted 8 lines
This is a classic information leak vulnerability. 'index' should be an unsigned int, given its usage above.
This patch is straight-forward; it changes 'index' to unsigned int in two switch-cases:
FBIOGETCMAP_SPARC&&FBIOPUTCMAP_SPARC.Signed-off-by Peter Malone <[email protected]>---
much better :)
Quoted 28 lines
v2: fixed formatting
drivers/video/fbdev/sbuslib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/video/fbdev/sbuslib.c +2
-3@@ -122,7 +122,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ured;unsigned char __user *ugreen;unsigned char __user *ublue; -int index, count, i; +unsigned int index, count, i; if (get_user(index, &c->index) || __get_user(count, &c->count) ||@@ -161,7 +161,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ugreen;unsigned char __user *ublue;struct fb_cmap *cmap = &info->cmap; -int index, count, i; +unsigned int index, count, i;u8 red, green, blue; if (get_user(index, &c->index) || --Quoted 2 lines
2.14.3
By just looking at the code and commit message:
Signature · 2 lines
Regards, Peter.
Bartlomiej Zolnierkiewicz
AppliedQuoted 1 line · Peter Malone
Hi folks,
Hi,
Quoted 5 lines
CVE-2018-6412 has been created for this. Is it possible for you to add a note indicating the CVE number when merging the patch?
I received the CVE number after the patch was created and ack'd, which is why I didn't include it in the commit message.
I queued the patch (with Mathieu's ACK and CVE number added to the patch description) for v4.16, thanks.
Quoted 58 lines · Mathieu Malaterre
Quoted 57 lines
Hi Peter,
Quoted 17 lines · Peter Malone
Fixing arbitrary kernel leak in case
FBIOGETCMAP_SPARCinsbusfb_ioctl_helper().'index' is defined as an int in
sbusfb_ioctl_helper(). We retrieve this from the user:
if (get_user(index, &c->index)||__get_user(count, &c->count) || __get_user(ured, &c->red) || __get_user(ugreen, &c->green) || __get_user(ublue, &c->blue)) return -EFAULT;and then we use 'index' in the following way:
red = cmap->red[index + i] >> 8;
green = cmap->green[index + i] >> 8;
blue = cmap->blue[index + i] >> 8;Quoted 8 lines
This is a classic information leak vulnerability. 'index' should be an unsigned int, given its usage above.
This patch is straight-forward; it changes 'index' to unsigned int in two switch-cases:
FBIOGETCMAP_SPARC&&FBIOPUTCMAP_SPARC.Signed-off-by Peter Malone <[email protected]>---
much better :)
Quoted 26 lines
v2: fixed formatting
drivers/video/fbdev/sbuslib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/video/fbdev/sbuslib.c +2
-2@@ -122,7 +122,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ured;unsigned char __user *ugreen;unsigned char __user *ublue; -int index, count, i; +unsigned int index, count, i; if (get_user(index, &c->index) || __get_user(count, &c->count) ||@@ -161,7 +161,7 @@ int sbusfb_ioctl_helper(unsigned long cmd, unsigned long arg,unsigned char __user *ugreen;unsigned char __user *ublue;struct fb_cmap *cmap = &info->cmap; -int index, count, i; +unsigned int index, count, i;u8 red, green, blue;
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
drivers/video/fbdev/sbuslib.c +0
-1if (get_user(index, &c->index) || --2.14.3
By just looking at the code and commit message:
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics