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: abbf9e8a45 ("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>
This commit is contained in:
ChenXiaoSong 2022-07-27 17:21:52 -07:00 committed by Darrick J. Wong
parent 4869b6e84a
commit 001c179c4e

View File

@ -439,29 +439,28 @@ xfs_getbmap(
whichfork = XFS_COW_FORK; whichfork = XFS_COW_FORK;
else else
whichfork = XFS_DATA_FORK; whichfork = XFS_DATA_FORK;
ifp = xfs_ifork_ptr(ip, whichfork);
xfs_ilock(ip, XFS_IOLOCK_SHARED); xfs_ilock(ip, XFS_IOLOCK_SHARED);
switch (whichfork) { switch (whichfork) {
case XFS_ATTR_FORK: case XFS_ATTR_FORK:
lock = xfs_ilock_attr_map_shared(ip);
if (!xfs_inode_has_attr_fork(ip)) if (!xfs_inode_has_attr_fork(ip))
goto out_unlock_iolock; goto out_unlock_ilock;
max_len = 1LL << 32; max_len = 1LL << 32;
lock = xfs_ilock_attr_map_shared(ip);
break; break;
case XFS_COW_FORK: case XFS_COW_FORK:
lock = XFS_ILOCK_SHARED;
xfs_ilock(ip, lock);
/* No CoW fork? Just return */ /* No CoW fork? Just return */
if (!ifp) if (!xfs_ifork_ptr(ip, whichfork))
goto out_unlock_iolock; goto out_unlock_ilock;
if (xfs_get_cowextsz_hint(ip)) if (xfs_get_cowextsz_hint(ip))
max_len = mp->m_super->s_maxbytes; max_len = mp->m_super->s_maxbytes;
else else
max_len = XFS_ISIZE(ip); max_len = XFS_ISIZE(ip);
lock = XFS_ILOCK_SHARED;
xfs_ilock(ip, lock);
break; break;
case XFS_DATA_FORK: case XFS_DATA_FORK:
if (!(iflags & BMV_IF_DELALLOC) && if (!(iflags & BMV_IF_DELALLOC) &&
@ -491,6 +490,8 @@ xfs_getbmap(
break; break;
} }
ifp = xfs_ifork_ptr(ip, whichfork);
switch (ifp->if_format) { switch (ifp->if_format) {
case XFS_DINODE_FMT_EXTENTS: case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE: case XFS_DINODE_FMT_BTREE: