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

[chenxiaosong (A)]

I will give some detailed code process.

firstly open():

```c
open
  do_sys_open
    do_sys_openat2
      do_filp_open
        path_openat
          open_last_lookups
            lookup_open
              atomic_open
                nfs_atomic_open
                  create_nfs_open_context
                    flags_to_mode() = FMODE_READ|FMODE_WRITE
                    alloc_nfs_open_context
                      ctx->mode = f_mode // FMODE_READ|FMODE_WRITE
                  // NFS_PROTO(dir)->open_context
                  nfs4_atomic_open
                    nfs4_do_open
                      _nfs4_do_open
                        fmode = _nfs4_ctx_to_openmode(ctx) = 3
                          ret = ctx->mode & (FMODE_READ|FMODE_WRITE) // 
ctx->mode = 3
                        nfs4_opendata_alloc
                          nfs4_map_atomic_open_share
                            return NFS4_SHARE_ACCESS_BOTH
```

secondly open():
```c
open
  do_sys_open
    do_sys_openat2
      do_filp_open
        path_openat
          open_last_lookups
            lookup_open
              return dentry // if (dentry->d_inode) {
          do_open
            vfs_open
              do_dentry_open
                // f->f_op->open
                nfs4_file_open
                  if ((openflags & O_ACCMODE) == 3)
                  nfs_open // without sunrpc request
                    alloc_nfs_open_context
                      ctx->state = NULL; // this is point
```

lseek() after secondly open():
```c
lseek
  ksys_lseek
    vfs_llseek
      // file->f_op->llseek
      nfs4_file_llseek
        nfs42_proc_llseek
          _nfs42_proc_llseek(lock)
            nfs4_set_rw_stateid(ctx=lock->open_context)
              nfs4_select_rw_stateid(state=ctx->state)
                nfs4_valid_open_stateid(state)
                  state->flags // dereference NULL state
```

在 2022/4/13 22:05, 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).