Skip to contentSkip to conversation

xfs: fix stack contents leakage in the v1 inumber ioctls

PATCH In review Darrick J. Wong 3 participants 3 messages Jul 25, 2019 – Jul 26, 2019 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 xfs: fix stack contents leakage in the v1 inumber ioctls

Explicitly initialize the onstack structures to zero so we don't leak kernel memory into userspace when converting the in-core inumbers structure to the v1 inogrp ioctl structure. Add a comment about why we have to use memset to ensure that the padding holes in the structures are set to zero.

Fixes 5f19c7fc6873351 ("xfs: introduce v5 inode group structure")

---
v2: add comments, use memset this time

---
 fs/xfs/xfs_itable.c |    3 +++
 1 file changed, 3 insertions(+)
fs/xfs/xfs_itable.c +3 -0
index 30fe17d25518..39374c680c49 100644
@@ -283,6 +283,7 @@ xfs_bulkstat_to_bstat(
 	struct xfs_bstat		*bs1,
 	const struct xfs_bulkstat	*bstat)
 {
+	/* memset is needed here because of padding holes in the structure. */
 	memset(bs1, 0, sizeof(struct xfs_bstat));
 	bs1->bs_ino = bstat->bs_ino;
 	bs1->bs_mode = bstat->bs_mode;
@@ -399,6 +400,8 @@ xfs_inumbers_to_inogrp(
 	struct xfs_inogrp		*ig1,
 	const struct xfs_inumbers	*ig)
 {
+	/* memset is needed here because of padding holes in the structure. */
+	memset(ig1, 0, sizeof(struct xfs_inogrp));
 	ig1->xi_startino = ig->xi_startino;
 	ig1->xi_alloccount = ig->xi_alloccount;
 	ig1->xi_allocmask = ig->xi_allocmask;
Reviewed-by on v2 1/1
#1 lore raw
Quoted 9 lines · Darrick J. Wong

Explicitly initialize the onstack structures to zero so we don't leak kernel memory into userspace when converting the in-core inumbers structure to the v1 inogrp ioctl structure. Add a comment about why we have to use memset to ensure that the padding holes in the structures are set to zero.

Fixes 5f19c7fc6873351 ("xfs: introduce v5 inode group structure")
Quoted 28 lines

---
v2: add comments, use memset this time

---
 fs/xfs/xfs_itable.c |    3 +++
 1 file changed, 3 insertions(+)
fs/xfs/xfs_itable.c +3 -0
index 30fe17d25518..39374c680c49 100644
@@ -283,6 +283,7 @@ xfs_bulkstat_to_bstat(
 	struct xfs_bstat		*bs1,
 	const struct xfs_bulkstat	*bstat)
 {
+	/* memset is needed here because of padding holes in the structure. */
 	memset(bs1, 0, sizeof(struct xfs_bstat));
 	bs1->bs_ino = bstat->bs_ino;
 	bs1->bs_mode = bstat->bs_mode;
@@ -399,6 +400,8 @@ xfs_inumbers_to_inogrp(
 	struct xfs_inogrp		*ig1,
 	const struct xfs_inumbers	*ig)
 {
+	/* memset is needed here because of padding holes in the structure. */
+	memset(ig1, 0, sizeof(struct xfs_inogrp));
 	ig1->xi_startino = ig->xi_startino;
 	ig1->xi_alloccount = ig->xi_alloccount;
 	ig1->xi_allocmask = ig->xi_allocmask;
Reviewed-by on v2 1/1
#2 lore raw
In reply to Darrick J. Wong
Quoted 11 lines · Darrick J. Wong

Explicitly initialize the onstack structures to zero so we don't leak kernel memory into userspace when converting the in-core inumbers structure to the v1 inogrp ioctl structure. Add a comment about why we have to use memset to ensure that the padding holes in the structures are set to zero.

Fixes 5f19c7fc6873351 ("xfs: introduce v5 inode group structure")

---
v2: add comments, use memset this time

Looks good,