Skip to contentSkip to conversation

xfs: Initialize label array properly

PATCH In review Austin Kim 3 participants 4 messages Aug 30, 2019 XFS filesystem

lore
Share & tools
lore

Austin Kim

author
#0 lore raw

PATCH 1/1 xfs: Initialize label array properly

In case kernel stack variable is not initialized properly, there is a risk of kernel information disclosure.

So, initialize 'char label[]' array with null characters.

---
 fs/xfs/xfs_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_ioctl.c +1 -1
index 9ea5166..09b3bee 100644
@@ -2037,7 +2037,7 @@ xfs_ioc_setlabel(
 	char			__user *newlabel)
 {
 	struct xfs_sb		*sbp = &mp->m_sb;
-	char			label[XFSLABEL_MAX + 1];
+	char			label[XFSLABEL_MAX + 1] = {0};
 	size_t			len;
 	int			error;
 
Signature · 1 line
2.6.2
#1 lore raw
Quoted 4 lines · Austin Kim

In case kernel stack variable is not initialized properly, there is a risk of kernel information disclosure.

So, initialize 'char label[]' array with null characters.

Got a testcase for this? At least a couple other filesystems implement this ioctl too, which means they all should be checked/tested on a regular basis.

--D

Quoted 21 lines
---
 fs/xfs/xfs_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_ioctl.c +1 -1
index 9ea5166..09b3bee 100644
@@ -2037,7 +2037,7 @@ xfs_ioc_setlabel(
 	char			__user *newlabel)
 {
 	struct xfs_sb		*sbp = &mp->m_sb;
-	char			label[XFSLABEL_MAX + 1];
+	char			label[XFSLABEL_MAX + 1] = {0};
 	size_t			len;
 	int			error;
 
Signature · 1 line
2.6.2
author
#2 lore raw

2019년 8월 30일 (금) 오후 2:39, Darrick J. Wong <[email protected]>님이 작성:

Quoted 10 lines · Austin Kim
Quoted 4 lines

In case kernel stack variable is not initialized properly, there is a risk of kernel information disclosure.

So, initialize 'char label[]' array with null characters.

Got a testcase for this? At least a couple other filesystems implement this ioctl too, which means they all should be checked/tested on a regular basis.

Thanks for feedback.
As you pointed out, I figured out ioctl(e.g: ext4_ioctl, f2fs_ioctl) of other file system is implemented with the same ways. Please ignore the patch in this mail thread.

BR,
Austin Kim

Quoted 24 lines

--D

Quoted 21 lines
---
 fs/xfs/xfs_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/xfs/xfs_ioctl.c +1 -2
index 9ea5166..09b3bee 100644
@@ -2037,7 +2037,7 @@ xfs_ioc_setlabel(
      char                    __user *newlabel)
 {
      struct xfs_sb           *sbp = &mp->m_sb;
-     char                    label[XFSLABEL_MAX + 1];
+     char                    label[XFSLABEL_MAX + 1] = {0};
      size_t                  len;
      int                     error;
 
--

2.6.2

#3 lore raw
In reply to Austin Kim
Quoted 4 lines · Austin Kim

In case kernel stack variable is not initialized properly, there is a risk of kernel information disclosure.

So, initialize 'char label[]' array with null characters.

Can you describe the information disclosure vector here? I can't see any, mostly because this is the "set label" function and that doesn't return anything to userspace.

We also zero the on-disk label before we copy the user label into it, so I don't see that anything can leak onto disk, either...

Cheers,

Dave.

Signature · 2 lines
Dave Chinner
[email protected]