* [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized
@ 2010-09-15 21:44 ` Dan Rosenberg
0 siblings, 0 replies; 6+ messages in thread
From: Dan Rosenberg @ 2010-09-15 21:44 UTC (permalink / raw)
To: JosephChan, ScottFang; +Cc: linux-fbdev, linux-kernel, security, stable
The VIAFB_GET_INFO device ioctl allows unprivileged users to read 1968
bytes of uninitialized stack memory, because the "reserved" member of
the viafb_ioctl_info struct declared on the stack is not altered or
zeroed before being copied back to the user. This patch takes care of
it.
Signed-off-by: Dan Rosenberg <dan.j.rosenberg@gmail.com>
--- linux-2.6.35.4.orig/drivers/video/via/ioctl.c 2010-08-26 19:47:12.000000000 -0400
+++ linux-2.6.35.4/drivers/video/via/ioctl.c 2010-09-15 11:53:29.997375748 -0400
@@ -25,6 +25,8 @@ int viafb_ioctl_get_viafb_info(u_long ar
{
struct viafb_ioctl_info viainfo;
+ memset(&viainfo, 0, sizeof(struct viafb_ioctl));
+
viainfo.viafb_id = VIAID;
viainfo.vendor_id = PCI_VIA_VENDOR_ID;
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized stack memory
@ 2010-09-15 21:44 ` Dan Rosenberg
0 siblings, 0 replies; 6+ messages in thread
From: Dan Rosenberg @ 2010-09-15 21:44 UTC (permalink / raw)
To: JosephChan, ScottFang; +Cc: linux-fbdev, linux-kernel, security, stable
The VIAFB_GET_INFO device ioctl allows unprivileged users to read 1968
bytes of uninitialized stack memory, because the "reserved" member of
the viafb_ioctl_info struct declared on the stack is not altered or
zeroed before being copied back to the user. This patch takes care of
it.
Signed-off-by: Dan Rosenberg <dan.j.rosenberg@gmail.com>
--- linux-2.6.35.4.orig/drivers/video/via/ioctl.c 2010-08-26 19:47:12.000000000 -0400
+++ linux-2.6.35.4/drivers/video/via/ioctl.c 2010-09-15 11:53:29.997375748 -0400
@@ -25,6 +25,8 @@ int viafb_ioctl_get_viafb_info(u_long ar
{
struct viafb_ioctl_info viainfo;
+ memset(&viainfo, 0, sizeof(struct viafb_ioctl));
+
viainfo.viafb_id = VIAID;
viainfo.vendor_id = PCI_VIA_VENDOR_ID;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized
2010-09-15 21:44 ` [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized stack memory Dan Rosenberg
@ 2010-09-15 22:42 ` Florian Tobias Schandinat
-1 siblings, 0 replies; 6+ messages in thread
From: Florian Tobias Schandinat @ 2010-09-15 22:42 UTC (permalink / raw)
To: Dan Rosenberg; +Cc: JosephChan, linux-fbdev, linux-kernel, security, stable
Hi,
Dan Rosenberg schrieb:
> The VIAFB_GET_INFO device ioctl allows unprivileged users to read 1968
> bytes of uninitialized stack memory, because the "reserved" member of
> the viafb_ioctl_info struct declared on the stack is not altered or
> zeroed before being copied back to the user. This patch takes care of
> it.
I am wondering about the number of bytes as sizeof(struct viafb_ioctl_info) =
256 so it should be a bit less. But that's just nitpicking, the issue is
certainly real. Your patch does the right thing but there is a bug in it:
> Signed-off-by: Dan Rosenberg <dan.j.rosenberg@gmail.com>
>
> --- linux-2.6.35.4.orig/drivers/video/via/ioctl.c 2010-08-26 19:47:12.000000000 -0400
> +++ linux-2.6.35.4/drivers/video/via/ioctl.c 2010-09-15 11:53:29.997375748 -0400
> @@ -25,6 +25,8 @@ int viafb_ioctl_get_viafb_info(u_long ar
> {
> struct viafb_ioctl_info viainfo;
>
> + memset(&viainfo, 0, sizeof(struct viafb_ioctl));
Shouldn't it be sizeof(struct viafb_ioctl_info) or sizeof(viainfo) ?
At least here it does not even compile otherwise....
Thanks,
Florian Tobias Schandinat
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized stack memory
@ 2010-09-15 22:42 ` Florian Tobias Schandinat
0 siblings, 0 replies; 6+ messages in thread
From: Florian Tobias Schandinat @ 2010-09-15 22:42 UTC (permalink / raw)
To: Dan Rosenberg; +Cc: JosephChan, linux-fbdev, linux-kernel, security, stable
Hi,
Dan Rosenberg schrieb:
> The VIAFB_GET_INFO device ioctl allows unprivileged users to read 1968
> bytes of uninitialized stack memory, because the "reserved" member of
> the viafb_ioctl_info struct declared on the stack is not altered or
> zeroed before being copied back to the user. This patch takes care of
> it.
I am wondering about the number of bytes as sizeof(struct viafb_ioctl_info) =
256 so it should be a bit less. But that's just nitpicking, the issue is
certainly real. Your patch does the right thing but there is a bug in it:
> Signed-off-by: Dan Rosenberg <dan.j.rosenberg@gmail.com>
>
> --- linux-2.6.35.4.orig/drivers/video/via/ioctl.c 2010-08-26 19:47:12.000000000 -0400
> +++ linux-2.6.35.4/drivers/video/via/ioctl.c 2010-09-15 11:53:29.997375748 -0400
> @@ -25,6 +25,8 @@ int viafb_ioctl_get_viafb_info(u_long ar
> {
> struct viafb_ioctl_info viainfo;
>
> + memset(&viainfo, 0, sizeof(struct viafb_ioctl));
Shouldn't it be sizeof(struct viafb_ioctl_info) or sizeof(viainfo) ?
At least here it does not even compile otherwise....
Thanks,
Florian Tobias Schandinat
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drivers/video/via/ioctl.c: prevent reading
2010-09-15 21:44 ` [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized stack memory Dan Rosenberg
@ 2010-09-16 16:07 ` Steven Rostedt
-1 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2010-09-16 16:07 UTC (permalink / raw)
To: Dan Rosenberg
Cc: JosephChan, ScottFang, linux-fbdev, linux-kernel, security,
stable
On Wed, Sep 15, 2010 at 05:44:19PM -0400, Dan Rosenberg wrote:
> The VIAFB_GET_INFO device ioctl allows unprivileged users to read 1968
> bytes of uninitialized stack memory, because the "reserved" member of
> the viafb_ioctl_info struct declared on the stack is not altered or
> zeroed before being copied back to the user. This patch takes care of
> it.
>
> Signed-off-by: Dan Rosenberg <dan.j.rosenberg@gmail.com>
>
> --- linux-2.6.35.4.orig/drivers/video/via/ioctl.c 2010-08-26 19:47:12.000000000 -0400
> +++ linux-2.6.35.4/drivers/video/via/ioctl.c 2010-09-15 11:53:29.997375748 -0400
> @@ -25,6 +25,8 @@ int viafb_ioctl_get_viafb_info(u_long ar
> {
> struct viafb_ioctl_info viainfo;
>
> + memset(&viainfo, 0, sizeof(struct viafb_ioctl));
> +
Again, this could probably be better off as:
- struct viafb_ioctl_info viainfo;
+ struct viafb_ioctl_info viainfo = {
+ .viafb_id = VIAID,
+ .vendor_id = PCI_VIA_VENDOR_ID,
+ };
- viainfo.viafb_id = VIAID;
- viainfo.vendor_id = PCI_VIA_VENDOR_ID;
-- Steve
> viainfo.viafb_id = VIAID;
> viainfo.vendor_id = PCI_VIA_VENDOR_ID;
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized stack memory
@ 2010-09-16 16:07 ` Steven Rostedt
0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2010-09-16 16:07 UTC (permalink / raw)
To: Dan Rosenberg
Cc: JosephChan, ScottFang, linux-fbdev, linux-kernel, security,
stable
On Wed, Sep 15, 2010 at 05:44:19PM -0400, Dan Rosenberg wrote:
> The VIAFB_GET_INFO device ioctl allows unprivileged users to read 1968
> bytes of uninitialized stack memory, because the "reserved" member of
> the viafb_ioctl_info struct declared on the stack is not altered or
> zeroed before being copied back to the user. This patch takes care of
> it.
>
> Signed-off-by: Dan Rosenberg <dan.j.rosenberg@gmail.com>
>
> --- linux-2.6.35.4.orig/drivers/video/via/ioctl.c 2010-08-26 19:47:12.000000000 -0400
> +++ linux-2.6.35.4/drivers/video/via/ioctl.c 2010-09-15 11:53:29.997375748 -0400
> @@ -25,6 +25,8 @@ int viafb_ioctl_get_viafb_info(u_long ar
> {
> struct viafb_ioctl_info viainfo;
>
> + memset(&viainfo, 0, sizeof(struct viafb_ioctl));
> +
Again, this could probably be better off as:
- struct viafb_ioctl_info viainfo;
+ struct viafb_ioctl_info viainfo = {
+ .viafb_id = VIAID,
+ .vendor_id = PCI_VIA_VENDOR_ID,
+ };
- viainfo.viafb_id = VIAID;
- viainfo.vendor_id = PCI_VIA_VENDOR_ID;
-- Steve
> viainfo.viafb_id = VIAID;
> viainfo.vendor_id = PCI_VIA_VENDOR_ID;
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-16 16:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-15 21:44 [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized Dan Rosenberg
2010-09-15 21:44 ` [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized stack memory Dan Rosenberg
2010-09-15 22:42 ` [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized Florian Tobias Schandinat
2010-09-15 22:42 ` [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized stack memory Florian Tobias Schandinat
2010-09-16 16:07 ` [PATCH] drivers/video/via/ioctl.c: prevent reading Steven Rostedt
2010-09-16 16:07 ` [PATCH] drivers/video/via/ioctl.c: prevent reading uninitialized stack memory Steven Rostedt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.