Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 21 Aug 2013 09:12:24 +0100
From: John Haxby <>
Subject: Re: Linux kernel: vfs_read()/vfs_write(): potential
 missing checks (or not?)

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.


Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.