From: Phillip Lougher <plougher@redhat.com>
Date: Thu, 2 Jun 2011 20:55:19 -0400
Subject: [fs] xfs: prevent leaking uninit stack memory in FSGEOMETRY_V1
Message-id: <20110602205519.5D52C44B5D@plougher.csb>
Patchwork-id: 4584
O-Subject: [kernel team] [RHEL5.7 PATCH 1/2] xfs: prevent leaking uninitialized stack memory in FSGEOMETRY_V1
Bugzilla: 677266
CVE: CVE-2011-0711

BZ #677266

Upsteam status:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=3a3675b7f23f83ca8c67c9c2b6edf707fd28d1ba

This is the first of two upstream patches which fix CVE-2011-0711
xfs: prevent leaking uninitialized stack memory in FSGEOMETRY_V1

The xfs FSGEOMETRY_v1 ioctl calls xfs_fs_geometry() with an
xfs_fsop_geom_v1_t structure allocated on the stack.  On x86_64 this
structure is padded by 4 bytes (108 bytes rounded to 112 bytes), and these bytes
are not initialized by xfs_fs_geometry() and are thus returned uninitialized to
user-space.

This fix is split across two patches upstream due to mistakes in the first
patch, which described the problem as:

"The FSGEOMETRY_V1 ioctl (and its compat equivalent) calls out to
xfs_fs_geometry() with a version number of 3.  This code path does not
fill in the logsunit member of the passed xfs_fsop_geom_t, leading to
the leaking of four bytes of uninitialized stack data to potentially
unprivileged callers.  Since all other members are filled in all code
paths and there are no padding bytes in this structure, it's safe to
avoid an expensive memset() in favor of just clearing this one field."

This is  confused...

The existing FSGEOMETRY_V1 ioctl function (xfs_ioc_fsgeometry_v1) calls
xfs_fs_geometry() with an xfs_fsop_geom_v1_t structure on the stack cast to a
xfs_fsop_geom_t structure, i.e.

        xfs_fsop_geom_v1_t      fsgeo;

        error = xfs_fs_geometry(mp, (xfs_fsop_geom_t *)&fsgeo, 3);

An xfs_fsop_geom_v1_t structure is the same as the xfs_fsop_geom structure
except it lacks the logsunit field at the end (i.e. the underlying size of the
xfs_fsop_geom_v1 structure is 108 bytes, the xfs_fsop_geom structure is 112
bytes, and logsunit is at the byte offset of 108).  So the fact the version
number 3 code path doesn't fill in logsunit is correct (as it will over-write
the stack).  On 32 bit machines this is entirely correct, there is no leak.
However, on x86_64 the xfs_fsop_geom_v1_t structure is padded to 112 bytes (4
bytes of padding), and this padding is returned to user-space unintialized.

This first patch is thus broken in that it over-writes the stack on
32-bit machines.

The second patch does the right thing, by converting all callers to pass a
xfs_fsop_geom_t structure, and then memsets the entire structure in
xfs_fs_geometry, avoiding padding issues on 64 bit machines, and stack
over-runs on 32 bit machines.

The compat ioctl (xfs_compat_ioc_fsgeometry_v1) allocates a xfs_fsop_geom on
the stack and passes this to xfs_fs_geometry, but when it is returned to the
user it is truncated to the size of the compat_xfs_fsop_geom_v1 structure.
This structure is _packed_ and it is 108 bytes in size.  Therefore the
uninitialised 4 bytes of padding is not returned to user-space.

Testing: existing FSGEOMETRY_V1 ioctl leak confirmed with test program, no
leaks found on patched kernel.

Original commit info

 From 919fe41c7921927f21ac0840a9d59df17f2b65c5 Mon Sep 17 00:00:00 2001
From: Dan Rosenberg <drosenberg@vsecurity.com>
Date: Mon, 14 Feb 2011 13:45:28 +0000
Subject: [PATCH 1/2] xfs: prevent leaking uninitialized stack memory in
FSGEOMETRY_V1

The FSGEOMETRY_V1 ioctl (and its compat equivalent) calls out to
xfs_fs_geometry() with a version number of 3.  This code path does not
fill in the logsunit member of the passed xfs_fsop_geom_t, leading to
the leaking of four bytes of uninitialized stack data to potentially
unprivileged callers.

v2 switches to memset() to avoid future issues if structure members
change, on suggestion of Dave Chinner.

Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
Reviewed-by: Eugene Teo <eugeneteo@kernel.org>
Signed-off-by: Alex Elder <aelder@sgi.com>
Signed-off-by: Phillip Lougher <plougher@redhat.com>

diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 561eaef..05d2171 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -56,6 +56,9 @@ xfs_fs_geometry(
 	xfs_fsop_geom_t		*geo,
 	int			new_version)
 {
+
+	memset(geo, 0, sizeof(*geo));
+
 	geo->blocksize = mp->m_sb.sb_blocksize;
 	geo->rtextsize = mp->m_sb.sb_rextsize;
 	geo->agblocks = mp->m_sb.sb_agblocks;