diff options
| author | ChenXiaoSong <chenxiaosong2@huawei.com> | 2022-07-27 17:21:52 -0700 |
|---|---|---|
| committer | Darrick J. Wong <djwong@kernel.org> | 2022-07-31 09:21:27 -0700 |
| commit | 001c179c4e26d04db8c9f5e3fef9558b58356be6 (patch) | |
| tree | 99fc0d1b7a8d0de290677245ef7a344a61031df2 /fs/xfs/xfs_bmap_util.c | |
| parent | 4869b6e84a23076eca813d5739d6717976aeae66 (diff) | |
| download | lwn-001c179c4e26d04db8c9f5e3fef9558b58356be6.tar.gz lwn-001c179c4e26d04db8c9f5e3fef9558b58356be6.zip | |
xfs: fix NULL pointer dereference in xfs_getbmap()
Reproducer:
1. fallocate -l 100M image
2. mkfs.xfs -f image
3. mount image /mnt
4. setxattr("/mnt", "trusted.overlay.upper", NULL, 0, XATTR_CREATE)
5. 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>
Diffstat (limited to 'fs/xfs/xfs_bmap_util.c')
| -rw-r--r-- | fs/xfs/xfs_bmap_util.c | 17 |
1 files changed, 9 insertions, 8 deletions
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index 74f96e1aa5cd..04d0c2bff67c 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); if (!xfs_inode_has_attr_fork(ip)) - goto out_unlock_iolock; + goto out_unlock_ilock; max_len = 1LL << 32; - lock = xfs_ilock_attr_map_shared(ip); break; case XFS_COW_FORK: + 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); break; case XFS_DATA_FORK: if (!(iflags & BMV_IF_DELALLOC) && @@ -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: |