On Wed, 20 Nov 2024 16:10:45 +0000, Al Viro wrote:
> On Wed, Nov 20, 2024 at 11:04:43AM +0800, Lizhi Xu wrote:
> > syzbot reported a null-ptr-deref in pick_link. [1]
> >
> > First, i_link and i_dir_seq are in the same union, they share the same memory
> > address, and i_dir_seq will be updated during the execution of walk_component,
> > which makes the value of i_link equal to i_dir_seq.
> >
> > Secondly, the chmod execution failed, which resulted in setting the mode value
> > of file0's inode to REG when executing ntfs_bad_inode.
> >
> > Third, when creating a symbolic link using the file0 whose inode has been marked
> > as bad, it is not determined whether its inode is bad, which ultimately leads to
> > null-ptr-deref when performing a mount operation on the symbolic link bus because
> > the i_link value is equal to i_dir_seq=2.
> >
> > Note: ("file0, bus" are defined in reproducer [2])
> >
> > To avoid null-ptr-deref in pick_link, when creating a symbolic link, first check
> > whether the inode of file is already bad.
>
> I would really like to understand how the hell did that bad inode end up passed
> to d_splice_alias()/d_instantiate()/whatever it had been.
1. In the move_mount() process, the inode is created by ntfs_alloc_inode() and enters d_splice_alias() by ntfs_lookup(), at this time inode is good, as shown below:
move_mount()->
user_path_at()->
filename_lookup()->
path_lookupat()->
lookup_last()->
walk_component()->
__lookup_slow()->
ntfs_lookup()->
d_splice_alias()->
2. The subsequent chmod fails, causing the inode to be set to bad.
3. During the link operation, d_instantiate() is executed in ntfs_link() to associate the bad inode with the dentry.
4. During the mount operation, walk_component executes pick_link, triggering null-ptr-deref.
Reproducer:
move_mount(0xffffffffffffff9c, &(0x7f00000003c0)='./file0\x00', 0xffffffffffffff9c, &(0x7f0000000400)='./file0/file0\x00', 0x140)
chmod(&(0x7f0000000080)='./file0\x00', 0x0)
link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00')
mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0)
>
> That's the root cause - and it looks like ntfs is too free with make_bad_inode()
> in general, which might cause other problems.
