Re: Linux kernel: vfs_read()/vfs_write(): potential missing checks (or not?)

[John Haxby <john.haxby () oracle com>]

On 20/08/13 23:36, Hannes Frederic Sowa wrote:
> On Tue, Aug 20, 2013 at 07:58:49PM +0200, vladz wrote:
> > > [...]
> > >
> > > Looking at the kernel sources, the vfs_read(), vfs_write(), vfs_readv()
> > > and vfs_writev() functions checks the permissions of the file object
> > > (file->f_mode) before operating on file descriptor:
> > >
> > >     $ cat -n linux-3.10.7/fs/read_write.c
> > >     [...]
> > >     353 ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
> > >     354 {
> > >     355         ssize_t ret;
> > >     356
> > >     357         if (!(file->f_mode & FMODE_READ))
> > >     358                 return -EBADF;
> > >
> > > I believe this is insufficient, the inode object should be checked too.
> > > So that if the file's permissions allow read/write operations, so we can
> > > perform reading/writing from/to the file descriptor.  I've patched the
> > > concerned function to do so (cf. patch [3]).
> This behavior is deliberatly chosen. If the inode is checked again, you
> could just mmap the filedescriptor to memory and get away with that,
> too. There are plans to implement a revoke-syscall. Maybe it will
> be implemented for files, too (other operating systems only provide
> revoke-Support for terminals, block or char devices).  This shoud then
> handle the teardown of memory mappings with some specified semantic, too.


If you want extended checking of read(2) then use selinux: it
specifically handles cases like this.

Checking the inode may result in perfectly reasonable behaviour suddenly
not working:

   my-suid-program > logfile

for instance will fail if the setuid owner doesn't have permission to
write to logfile which would surprise a lot of people (and break a lot
of things).   Moreover,

  my-suid-program | tee logfile >/dev/null

would work.   On the other hand, selinux takes care of both of these: it
only forbids the write() when it's wrong will (often) track the write
across a pipeline.

jch