Message ID | 1309796566-2645-3-git-send-email-paolo.pisati@canonical.com |
---|---|
State | New |
Headers | show |
On Mon, Jul 04, 2011 at 05:22:42PM +0100, paolo.pisati@canonical.com wrote: > From: Alex Elder <aelder@sgi.com> > > BugLink: http://bugs.launchpad.net/bugs/767740 > > commit upstream af24ee9ea8d532e16883251a6684dfa1be8eec29 > > Commit 493f3358cb289ccf716c5a14fa5bb52ab75943e5 added this call to > xfs_fs_geometry() in order to avoid passing kernel stack data back > to user space: > > + memset(geo, 0, sizeof(*geo)); > > Unfortunately, one of the callers of that function passes the > address of a smaller data type, cast to fit the type that > xfs_fs_geometry() requires. As a result, this can happen: > > Kernel panic - not syncing: stack-protector: Kernel stack is corrupted > in: f87aca93 > > Pid: 262, comm: xfs_fsr Not tainted 2.6.38-rc6-493f3358cb2+ #1 > Call Trace: > > [<c12991ac>] ? panic+0x50/0x150 > [<c102ed71>] ? __stack_chk_fail+0x10/0x18 > [<f87aca93>] ? xfs_ioc_fsgeometry_v1+0x56/0x5d [xfs] > > Fix this by fixing that one caller to pass the right type and then > copy out the subset it is interested in. > > Note: This patch is an alternative to one originally proposed by > Eric Sandeen. > > CVE-2011-0711 > > Reported-by: Jeffrey Hundstad <jeffrey.hundstad@mnsu.edu> > Signed-off-by: Alex Elder <aelder@sgi.com> > Reviewed-by: Eric Sandeen <sandeen@redhat.com> > Tested-by: Jeffrey Hundstad <jeffrey.hundstad@mnsu.edu> > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> > --- > fs/xfs/linux-2.6/xfs_ioctl.c | 11 ++++++++--- > 1 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c > index 2f79328..f70a1a7 100644 > --- a/fs/xfs/linux-2.6/xfs_ioctl.c > +++ b/fs/xfs/linux-2.6/xfs_ioctl.c > @@ -1081,14 +1081,19 @@ xfs_ioc_fsgeometry_v1( > xfs_mount_t *mp, > void __user *arg) > { > - xfs_fsop_geom_v1_t fsgeo; > + xfs_fsop_geom_t fsgeo; > int error; > > - error = xfs_fs_geometry(mp, (xfs_fsop_geom_t *)&fsgeo, 3); > + error = xfs_fs_geometry(mp, &fsgeo, 3); > if (error) > return -error; > > - if (copy_to_user(arg, &fsgeo, sizeof(fsgeo))) > + /* > + * Caller should have passed an argument of type > + * xfs_fsop_geom_v1_t. This is a proper subset of the > + * xfs_fsop_geom_t that xfs_fs_geometry() fills in. > + */ > + if (copy_to_user(arg, &fsgeo, sizeof(xfs_fsop_geom_v1_t))) > return -XFS_ERROR(EFAULT); > return 0; > } Appears to do what it claims, and is identicle to the upstream commit. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c index 2f79328..f70a1a7 100644 --- a/fs/xfs/linux-2.6/xfs_ioctl.c +++ b/fs/xfs/linux-2.6/xfs_ioctl.c @@ -1081,14 +1081,19 @@ xfs_ioc_fsgeometry_v1( xfs_mount_t *mp, void __user *arg) { - xfs_fsop_geom_v1_t fsgeo; + xfs_fsop_geom_t fsgeo; int error; - error = xfs_fs_geometry(mp, (xfs_fsop_geom_t *)&fsgeo, 3); + error = xfs_fs_geometry(mp, &fsgeo, 3); if (error) return -error; - if (copy_to_user(arg, &fsgeo, sizeof(fsgeo))) + /* + * Caller should have passed an argument of type + * xfs_fsop_geom_v1_t. This is a proper subset of the + * xfs_fsop_geom_t that xfs_fs_geometry() fills in. + */ + if (copy_to_user(arg, &fsgeo, sizeof(xfs_fsop_geom_v1_t))) return -XFS_ERROR(EFAULT); return 0; }