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@xxxxxxxxxxxxxx> 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
