NVME_IOCTL_SUBMIT_IO access control

[Keith Busch]

On Tue, Jun 01, 2021 at 03:15:14PM +0000, Niklas Cassel wrote:
> On Tue, Jun 01, 2021 at 06:53:10AM -0700, Keith Busch wrote:
> > On Mon, May 31, 2021 at 11:09:35AM +0000, Niklas Cassel wrote:
> > > Hello there,
> > > 
> > > How is the NVME_IOCTL_SUBMIT_IO access control supposed to work?
> > > 
> > > $ echo "hello" | nvme write /dev/nvme0n1 -s 0 -c 1 -z 512
> > > /dev/nvme0n1: Permission denied
> > > 
> > > $ sudo chmod o+r /dev/nvme0n1
> > > 
> > > $ echo "hello" | nvme write /dev/nvme0n1 -s 0 -c 1 -z 512
> > > Rounding data size to fit block count (8192 bytes)
> > > write: Success
> > > 
> > > 
> > > Am I supposed to be able to do a write if I only have read permission?
> > > 
> > > $ ls -al /dev/nvme0n1
> > > brw-rw-r-- 1 root disk 259, 0 May 31 10:59 /dev/nvme0n1
> > 
> > I'm not sure, we've always allowed any user passthrough command if
> > CAP_SYS_ADMIN capable. It should be pretty easy to check permissions for
> > any data-out opcode against FMODE_WRITE. We'll probably break something
> > if we do, though.
> 
> Hello Keith,
> 
> Indeed,
> NVME_IOCTL_IO_CMD and NVME_IOCTL_IO64_CMD does require CAP_SYS_ADMIN,
> however, the same is not true for NVME_IOCTL_SUBMIT_IO.
> 
> Which is why as a regular user, with only o+r, I could do a write using
> NVME_IOCTL_SUBMIT_IO.
> 
> In one way it makes sense that NVME_IOCTL_IO_CMD requires CAP_SYS_ADMIN,
> since you can send any command to the drive using that ioctl.
> 
> With NVME_IOCTL_SUBMIT_IO, you can only submit write/read/compare.
> So it makes sense to not need CAP_SYS_ADMIN.
> 
> But I probably would expect data-out to be checked against FMODE_WRITE.
> 
> I just wanted to raise the issue, in case this was accidentally overlooked.

You're right, and SUBMIT_IO has always behaved that way. I believe this
was an oversight and should have been checking fmode_t against the
opcode.