xfs: prevent leaking uninitialized stack memory in FSGEOMETRY_V1
PATCH 1/1 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. 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.
--- fs/xfs/xfs_fsops.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
fs/xfs/xfs_fsops.c +1 -0
@@ -102,6 +102,7 @@ xfs_fs_geometry( mp->m_sb.sb_logsectsize : BBSIZE; geo->rtsectsize = mp->m_sb.sb_blocksize; geo->dirblocksize = mp->m_dirblksize; + geo->logsunit =0 ; }if (new_version >=4 ) { geo->flags |=
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 9 lines · Dan Rosenberg
The
FSGEOMETRY_V1ioctl (and its compat equivalent) calls out toxfs_fs_geometry()with a version number of 3. This code path does not fill in the logsunit member of the passedxfs_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 expensivememset()in favor of just clearing this one field.Signed-off-by Dan Rosenberg <[email protected]>
There are three callers to xfs_fs_geometry() with version number 3 and
4. I don't see any for version number 2, so this looks fine.
Thanks, Eugene
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Dan Rosenberg
The
FSGEOMETRY_V1ioctl (and its compat equivalent) calls out toxfs_fs_geometry()with a version number of 3. This code path does not fill in the logsunit member of the passedxfs_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 expensivememset()in favor of just clearing this one field.
If this really is a security problem, then it should use a memset. This is not a performance critical path and there are differences in the padding of the structure between 32 bit and 64 bit ioctl variants (it has a compat ioctl handler) and that can only be correctly handled by memset().
Also, using a memset means we won't have the problem of introducing new uninitialised fields or padding if we ever rev the structure again...
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH v2 1/1 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.
--- fs/xfs/xfs_fsops.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
fs/xfs/xfs_fsops.c +3 -0
@@ -53,6 +53,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;
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 10 lines · Dan Rosenberg
The
FSGEOMETRY_V1ioctl (and its compat equivalent) calls out toxfs_fs_geometry()with a version number of 3. This code path does not fill in the logsunit member of the passedxfs_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 <[email protected]>
Quoted 21 lines
---
fs/xfs/xfs_fsops.c | 3 +++1 files changed, 3 insertions(+), 0 deletions(-)fs/xfs/xfs_fsops.c +3
-1@@ -53,6 +53,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; --
Quoted 5 lines
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs