Re: Information Leak: FIDEDUPERANGE ioctl allows reading writeonly files

[Josef Bacik <josef@toxicpanda.com>]

On Tue, Jul 12, 2022 at 12:07:46PM -0700, Linus Torvalds wrote:
> On Tue, Jul 12, 2022 at 12:02 PM Josef Bacik <josef@toxicpanda.com> wrote:
> >
> > > Any permission checks done at IO time are basically always buggy:
> > > things may have changed since the 'open()', and those changes
> > > explicitly should *not* matter for the IO. That's really fundamentally
> > > how UNIX file permissions work.
> >
> > I don't think we should go this far, after all the normal
> > write()/read() syscalls do the permission checking each time as well,
> 
> No, they really don't.
> 
> The permission check is ONLY DONE AT OPEN TIME.
> 
> Really. Go look.
>

I did, I just misread what rw_verify_area was doing, it's just doing the
security check, not a full POSIX permissions check, my mistake.

> Anything else is a bug. If you open a file, and then change the
> permissions of the file (or the ownership, or whatever) afterwards,
> the open file descriptor is still supposed to be readable or writable.
> 
> Doing IO time permission checks is not only wrong, it's ACTIVELY
> BUGGY, and is a fairly common source of security problems (ie passing
> a file descriptor off to a suid binary, and then using the suid
> permissions to make changes that the original opener didn't have the
> rights to do).
> 
> So if you do permission checks at read/write time, you are a buggy
> mess.  It really is that simple.
> 
> This is why read and write check FMODE_READ and FMODE_WRITE. That's
> the *open* time check.
> 
> The fact that dedupe does that inode_permission() check at IO time
> really looks completely bogus and buggy.
> 

Yeah I'm fine with removing the inode_permission(), I just want to make sure
we're consistent across all of our IO related operations.  It looks like at the
very least we're getting security_*_permission on things like
read/write/fallocate, so perhaps switch to that here and call it good enough?
Thanks,

Josef