On Thu, Mar 10, 2016 at 10:33:49AM -0800, Linus Torvalds wrote: > On Thu, Mar 10, 2016 at 6:58 AM, Ric Wheeler <ricwheeler@xxxxxxxxx> wrote: > > > > What was objectionable at the time this patch was raised years back (not > > just to me, but to pretty much every fs developer at LSF/MM that year) > > centered on the concern that this would be viewed as a "performance" mode > > and we get pressure to support this for non-priveleged users. It gives any > > user effectively the ability to read the block device content for previously > > allocated data without restriction. Sure, but it was never "any user". We always had group-based permissions from the beginning. Sure, we passed it in via a mount option which was a bit hacky, but we never got to the point of discussing the way that we would modulate the access --- the complaint seemed to be that if it was a non-root user, it was an unacceptable security hole. And the pushback I got was more in the way of a religious objection more than anything else. Heck, even reserving a code point for the out-of-tree patch received a huge amount of pushback. > The sane way to do it would be to just check permissions of the > underlying block device. > > That way, people can just set the permissions for that to whatever > they want. If google right now uses some magical group for this, they > could make the underlying block device be writable for that group. I'd suggest making it be if you had *read* access to the block device. After all, the risk that everyone was all excited about was the risk of being able to read stale (deleted) data from old files. And there's no point giving the userspace cluster file system daemon the ability to corrupt the file system or set the setuid bit on some arbitrary executable. And if we are going to go this far, then I'd also suggest using this permission check to the user the ability to issue BLKDISCARD on a file. Allowing BLKDISCARD on files is one that should have been even more of a no-brainer, since it could never reveal stale data, but simply wasn't guaranteed to have reliable results because it was a hint to the underlying storage device. But this has also received a huge amount of religious pushback, which is why this is also an out-of-tree patch in the Google kernel. (If that means that our competitors have a higher flash TCO than us, again, no skin off my nose. I tried to get it upstream, and cost of forward porting the patch each time we rebase the kernel isn't _that_ annoying.) - Ted > We can do the security check at the filesystem level, because we have > sb->s_bdev->bd_inode, and if you have read and write permissions to > that inode, you might as well have permission to create a unsafe hole. > > That doesn't sound very hacky to me. > > Linus -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
