Re: [PATCH -next 0/2] fix nfsv4 bugs of opening with O_ACCMODE flag

[chenxiaosong (A)]

在 2022/4/13 21:42, chenxiaosong (A) 写道:
> 在 2022/4/13 20:07, Lyu Tao 写道:
> > Hi Xiaosong,
> >
> >
> > Thanks for keeping focusing on this bug.
> >
> >
> > I applied this CVE for the NULL dereference bug at 
> > nfs4_valid_open_stateid() and added the following description to this 
> > CVE due to the NFS maintainers replied that to me.
> >
> > "An issue was discovered in fs/nfs/dir.c in the Linux kernel before 
> > 5.16.5. If an application sets the O_DIRECTORY flag, and tries to open 
> > a regular file, nfs_atomic_open() performs a regular lookup. If a 
> > regular file is found, ENOTDIR should occur, but the server instead 
> > returns uninitialized data in the file descriptor.
> >
> >
> > Actually I'm still confused with the root cause of this bug. In the 
> > original PoC, there is no O_DIRECTORY flag but commit ac795161c936 
> > mentioned.
> >
> > Moreover, in your latest commit ab0fc21bc710, it said "After secondly 
> > opening a file with O_ACCMODE|O_DIRECT flags, 
> > nfs4_valid_open_stateid() will dereference NULL nfs4_state when 
> > lseek()." However, the original PoC opens the file only with 
> > O_RDWR|O_CREAT for the first time.
> >
> >
> > Original PoC:
> >
> > fd = openat("./file1", o_RDWR|O_CREAT, 000);
> >
> > open("./file1", 
> > O_ACCMODE|O_CREAT|O_DIRECT|O_LARGEFILE|O_NOFOLLOW|O_NOATIME|O_CLOEXEC|FASYNC|0xb3000008, 
> > 001);
> >
> > lseek(fd, 9, SEEK_HOLE);
> >
> >
> > I'll update this CVE's description after I figure out these.
> >
> >
> > Best Regards,
> >
> > Tao
>
> Hi Tao:
>
> Yes, O_ACCEMODE is _not_ necessary when fistly open() file.
>
> When open() the file secondly, O_ACCEMODE is necessary if we want to 
> reproduce the bug.
>
> Waiting for your modification of the CVE's description.
>
> Best Regards.
> .

My reproducer:
  1. mount -t nfs -o vers=4.2 $server_ip:/ /mnt/
  2. fd = open("/mnt/file", O_ACCMODE|O_DIRECT|O_CREAT)
  3. close(fd)
  4. fd = open("/mnt/file", O_ACCMODE|O_DIRECT)
  5. lseek(fd)

When firstly open() file, O_ACCMODE|O_DIRECT is _not_ necessary, we just 
use O_CREAT to create new file.

When secondly open() file, only O_ACCMODE|O_DIRECT is necessary, 
O_CREAT|O_LARGEFILE|O_NOFOLLOW|O_NOATIME|O_CLOEXEC|FASYNC|0xb3000008 in 
your original PoC is not necessary (however, they are harmless).