On Wed, Nov 15, 2023 at 06:28:25PM -0800, Leah Rumancik wrote:
From: ChenXiaoSong chenxiaosong2@huawei.com
[ Upstream commit 001c179c4e26d04db8c9f5e3fef9558b58356be6 ]
Reproducer:
- fallocate -l 100M image
- mkfs.xfs -f image
- mount image /mnt
- setxattr("/mnt", "trusted.overlay.upper", NULL, 0, XATTR_CREATE)
- char arg[32] = "\x01\xff\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00" "\x00\x00\x00\x00\x00\x08\x00\x00\x00\xc6\x2a\xf7"; fd = open("/mnt", O_RDONLY|O_DIRECTORY); ioctl(fd, _IOC(_IOC_READ|_IOC_WRITE, 0x58, 0x2c, 0x20), arg);
NULL pointer dereference will occur when race happens between xfs_getbmap() and xfs_bmap_set_attrforkoff():
ioctl | setxattr----------------------------|--------------------------- xfs_getbmap | xfs_ifork_ptr | xfs_inode_has_attr_fork | ip->i_forkoff == 0 | return NULL | ifp == NULL | | xfs_bmap_set_attrforkoff | ip->i_forkoff > 0 xfs_inode_has_attr_fork | ip->i_forkoff > 0 | ifp == NULL | ifp->if_format |
Fix this by locking i_lock before xfs_ifork_ptr().
Fixes: abbf9e8a4507 ("xfs: rewrite getbmap using the xfs_iext_* helpers") Signed-off-by: ChenXiaoSong chenxiaosong2@huawei.com Signed-off-by: Guo Xuenan guoxuenan@huawei.com Reviewed-by: Darrick J. Wong djwong@kernel.org [djwong: added fixes tag] Signed-off-by: Darrick J. Wong djwong@kernel.org Signed-off-by: Leah Rumancik leah.rumancik@gmail.com Acked-by: Chandan Babu R chandanbabu@kernel.org
fs/xfs/xfs_bmap_util.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index fd2ad6a3019c..bea6cc26abf9 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -439,29 +439,28 @@ xfs_getbmap( whichfork = XFS_COW_FORK; else whichfork = XFS_DATA_FORK;
- ifp = XFS_IFORK_PTR(ip, whichfork);
xfs_ilock(ip, XFS_IOLOCK_SHARED); switch (whichfork) { case XFS_ATTR_FORK:
lock = xfs_ilock_attr_map_shared(ip);
goto out_unlock_iolock;
goto out_unlock_ilock;max_len = 1LL << 32;
lock = xfs_ilock_attr_map_shared(ip);
lock = XFS_ILOCK_SHARED;xfs_ilock(ip, lock);- /* No CoW fork? Just return */
if (!ifp)goto out_unlock_iolock;
if (!XFS_IFORK_PTR(ip, whichfork))goto out_unlock_ilock;if (xfs_get_cowextsz_hint(ip)) max_len = mp->m_super->s_maxbytes; else max_len = XFS_ISIZE(ip);
lock = XFS_ILOCK_SHARED;xfs_ilock(ip, lock);@@ -491,6 +490,8 @@ xfs_getbmap( break; }
- ifp = XFS_IFORK_PTR(ip, whichfork);
- switch (ifp->if_format) { case XFS_DINODE_FMT_EXTENTS: case XFS_DINODE_FMT_BTREE:
-- 2.43.0.rc0.421.g78406f8d94-goog
This patch breaks the build, how was it tested?
fs/xfs/xfs_bmap_util.c: In function ‘xfs_getbmap’: fs/xfs/xfs_bmap_util.c:457:21: error: the comparison will always evaluate as ‘true’ for the address of ‘i_df’ will never be NULL [-Werror=address] 457 | if (!XFS_IFORK_PTR(ip, whichfork)) | ^ In file included from fs/xfs/xfs_bmap_util.c:16: fs/xfs/xfs_inode.h:38:33: note: ‘i_df’ declared here 38 | struct xfs_ifork i_df; /* data fork */ | ^~~~ cc1: all warnings being treated as errors