Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1354530 > unrolled thread

Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Gregory Farnum <greg@gregs42.com> - 2016-03-09 23:30 +0100
    Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-10 00:10 +0100
      Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Ric Wheeler <ricwheeler@gmail.com> - 2016-03-10 16:00 +0100
        Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-10 19:40 +0100
          Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-10 22:50 +0100
          Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Ric Wheeler <rwheeler@redhat.com> - 2016-03-11 05:50 +0100
            Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range  of blocks One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-03-11 15:10 +0100
              Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-11 16:30 +0100
              Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-11 18:30 +0100
                Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Andy Lutomirski <luto@amacapital.net> - 2016-03-11 18:40 +0100
                  Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-11 19:30 +0100
                    Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Dave Chinner <david@fromorbit.com> - 2016-03-11 23:40 +0100
                      Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-12 01:40 +0100
                        Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-12 01:50 +0100
                          Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-12 08:30 +0100
                            Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Thomas Schoebel-Theuer <tst@schoebel-theuer.de> - 2016-03-12 11:20 +0100
                          Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Dave Chinner <david@fromorbit.com> - 2016-03-14 00:40 +0100
                            Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Ric Wheeler <rwheeler@redhat.com> - 2016-03-14 11:40 +0100
                              Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-14 15:50 +0100
                                Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Dave Chinner <david@fromorbit.com> - 2016-03-15 21:20 +0100
                                  Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-15 21:50 +0100
                                    Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-15 22:30 +0100
                                    Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Dave Chinner <david@fromorbit.com> - 2016-03-15 23:40 +0100
                                      Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-16 00:00 +0100
                                        Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-03-16 03:00 +0100
                                          Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Andreas Dilger <adilger@dilger.ca> - 2016-03-16 22:50 +0100
                                            Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-17 01:20 +0100
                                              Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Eric Sandeen <esandeen@redhat.com> - 2016-03-17 01:40 +0100
                                                Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-17 02:00 +0100
                                                Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Gregory Farnum <greg@gregs42.com> - 2016-03-17 06:20 +0100
                                                  Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Theodore Ts'o <tytso@mit.edu> - 2016-03-17 13:40 +0100
                                          Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Dave Chinner <david@fromorbit.com> - 2016-03-17 02:10 +0100
                                            Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-03-17 03:50 +0100
                                      Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-16 00:10 +0100
                                        Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-16 00:20 +0100
                                          Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Dave Chinner <david@fromorbit.com> - 2016-03-16 01:10 +0100
                                        Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Dave Chinner <david@fromorbit.com> - 2016-03-16 01:00 +0100
                                          Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-16 01:10 +0100
                                            Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Eric Sandeen <esandeen@redhat.com> - 2016-03-16 01:40 +0100
                                              Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Chris Mason <clm@fb.com> - 2016-03-16 02:00 +0100
                                                Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Chris Mason <clm@fb.com> - 2016-03-16 23:30 +0100
                                                  Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Ric Wheeler <rwheeler@redhat.com> - 2016-03-17 14:50 +0100
                                  Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of  blocks Eric Sandeen <esandeen@redhat.com> - 2016-03-15 23:40 +0100
                      Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-12 01:40 +0100

Page 1 of 3  [1] 2 3  Next page →

#1354530 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Thu, Mar 3, 2016 at 3:10 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Thu, Mar 03, 2016 at 05:39:52PM -0500, Theodore Ts'o wrote:
>> On Thu, Mar 03, 2016 at 01:54:54PM -0500, Martin K. Petersen wrote:
>> > >>>>> "Christoph" == Christoph Hellwig <hch@infradead.org> writes:
>> >
>> > Christoph>  - FALLOC_FL_PUNCH_HOLE assures zeroes are returned, but
>> > Christoph> space is deallocated as much as possible -
>> > Christoph> FALLOC_FL_ZERO_RANGE assures zeroes are returned, AND blocks
>> > Christoph> are actually allocated
>> >
>> > That works for me. I think it would be great if we could have consistent
>> > interfaces for fs and block. The more commonality the merrier.
>>
>> So a question I have is do we want to add a "discard-as-a-hint" analog
>> for fallocate?
>
> Well defined, reliable behaviour only, please. If the device can't
> provide the required hardware offload, then it needs to use the
> generic, slow implementation of the functionality or report
> EOPNOTSUPP.
>
>> P.S.  Speaking of things that are powerful and too dangerous for
>> application programmers, after the Linux FAST workshop, I was having
>> dinner with the Ceph developers and Ric Wheeler, and we were talking
>> about things they really needed.  Turns out they also could use an
>> FALLOC_FL_NO_HIDE_STALE functionality.
>
> For better or for worse, Ceph is moving away from using filesystems
> for its back end object store, so the use of such a hack in Ceph
> has a very limited life.

Well, let's be clear: the reason Ceph is moving away from using local
filesystems is because we couldn't get the overheads of using them
down to what we considered an acceptable level. There are always going
to be some inefficiencies from it of course (since you have two
metadata streams) but the more issues get addressed, the fewer
userspace filesystems will feel or run up against the need to do their
own block device management. :) If none of them get fixed the same
scenario will just repeat itself — a userspace filesystem rises, it
tries to get features it needs into the kernel, it eventually gives up
and drops the kernel out of the loop, and then the fact that nobody's
using the kernel in this scenario will be considered a reason not to
make it work better.

I really am sensitive to the security concerns, just know that if it's
a permanent blocker you're essentially blocking out a growing category
of disk users (who run on an awfully large number of disks!).
-Greg

>
>> I told them I had an
>> out-of-tree patch that had that functionality, and even Ric Wheeler
>> started getting tempted....  :-)
>
> You can tempt all you want, but it does not change the basic fact
> that it is dangerous and compromises system security. As such, it
> does not belong in upstream kernels. Especially in this day and age
> where ensuring the fundamental integrity of our systems is more
> important than ever.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [next] | [standalone]

#1354543 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Wed, Mar 09, 2016 at 02:20:31PM -0800, Gregory Farnum wrote:
> I really am sensitive to the security concerns, just know that if it's
> a permanent blocker you're essentially blocking out a growing category
> of disk users (who run on an awfully large number of disks!).

Or they just have to use kernels with out-of-tree patches installed.  :-P

If you want to consider how many disks Google has that are using this
patch, I probably could have appealed to Linus and asked him to accept
the patch if I forced the issue.  The only reason why I didn't was
that people like Ric Wheeler threatened to have distro-specific
patches to disable the feature, and at the end of the day, I didn't
care that much.  After all, if it makes it harder for large scale
cloud companies besides Google to create more efficient userspace
cluster file systems, it's not like I was keeping the patch a secret.

So ultimately, if the Ceph developers want to make a case to Red Hat
management that this is important, great.  If not, it's not that hard
for those people who need the patch and who are running large cloud
infrastructures to simply apply the out-of-tree patch if they need it.

Cheers,

   	     	     	 	      		  - Ted

[toc] | [prev] | [next] | [standalone]

#1355176 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On 03/10/2016 04:38 AM, Theodore Ts'o wrote:
> On Wed, Mar 09, 2016 at 02:20:31PM -0800, Gregory Farnum wrote:
>> I really am sensitive to the security concerns, just know that if it's
>> a permanent blocker you're essentially blocking out a growing category
>> of disk users (who run on an awfully large number of disks!).
> Or they just have to use kernels with out-of-tree patches installed.  :-P
>
> If you want to consider how many disks Google has that are using this
> patch, I probably could have appealed to Linus and asked him to accept
> the patch if I forced the issue.  The only reason why I didn't was
> that people like Ric Wheeler threatened to have distro-specific
> patches to disable the feature, and at the end of the day, I didn't
> care that much.  After all, if it makes it harder for large scale
> cloud companies besides Google to create more efficient userspace
> cluster file systems, it's not like I was keeping the patch a secret.
>
> So ultimately, if the Ceph developers want to make a case to Red Hat
> management that this is important, great.  If not, it's not that hard
> for those people who need the patch and who are running large cloud
> infrastructures to simply apply the out-of-tree patch if they need it.
>
> Cheers,
>
>     	     	     	 	      		  - Ted
>

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.

At the time, I also don't recall seeing the patch posted on upstream lists for 
debate or justification.

As we discussed a few weeks back, I don't object to having support for doing 
this in carefully controlled ways for things like user space file systems. In 
effect, the problem of preventing other people's data being handed over to the 
end user is taken on by that layer of code. I suspect that fits the use case at 
google and Ceph both.

Regards,

Ric

[toc] | [prev] | [next] | [standalone]

#1355312

On Thu, Mar 10, 2016 at 6:58 AM, Ric Wheeler <ricwheeler@gmail.com> 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.

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.

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

[toc] | [prev] | [next] | [standalone]

#1355419 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

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@gmail.com> 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

[toc] | [prev] | [next] | [standalone]

#1355609 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On 03/11/2016 12:03 AM, Linus Torvalds wrote:
> On Thu, Mar 10, 2016 at 6:58 AM, Ric Wheeler <ricwheeler@gmail.com> 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.
> 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.
>
> 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

I agree that this sounds quite reasonable.

Ric

[toc] | [prev] | [next] | [standalone]

#1355929 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

> > 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.

Not if you don't have access to a block device node to open it, or there
are SELinux rules that control the access. There are cases it isn't
entirely the same thing as far as I can see. Consider within a container
for example.

The paranoid approach would IMHO to have a mount option so you can
explicitly declare a file system mount should trust its owner/group and
then that can also be used to wire up any other "unsafe" activities in a
general "mounted for a special use" option.

Alan

[toc] | [prev] | [next] | [standalone]

#1355981 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Fri, Mar 11, 2016 at 01:59:52PM +0000, One Thousand Gnomes wrote:
> > > 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.
> 
> Not if you don't have access to a block device node to open it, or there
> are SELinux rules that control the access. There are cases it isn't
> entirely the same thing as far as I can see. Consider within a container
> for example.

In a container shouldn't be a problem so long as we use uid mapping
when making the group id check.

> The paranoid approach would IMHO to have a mount option so you can
> explicitly declare a file system mount should trust its owner/group and
> then that can also be used to wire up any other "unsafe" activities in a
> general "mounted for a special use" option.

Indeed, that's what we're currently doing.  We've acutally been using
different gid's for each "privileged" operation, though, since we want
to have fine-grained access controls.

The process who can perform an operation which can result in the
ability to read stale data might not need (and therefore should not be
given) access to be able to issue TCG/Opal management commands to the
HDD, for example.

	 	    	     	    	 - Ted

[toc] | [prev] | [next] | [standalone]

#1356084

On Fri, Mar 11, 2016 at 5:59 AM, One Thousand Gnomes
<gnomes@lxorguk.ukuu.org.uk> wrote:
>
> > > 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.
>
> Not if you don't have access to a block device node to open it, or there
> are SELinux rules that control the access. There are cases it isn't
> entirely the same thing as far as I can see. Consider within a container
> for example.

I agree that it's not the same thing, but I don't think it really ends
up mattering.

Either the container is properly separated and set up - in which case
the uid mapping is what protects you - or it isn't - in which case the
container could just mknod whatever hell node it wants anyway.

So we do pretty much have the permission model.

> The paranoid approach would IMHO to have a mount option so you can
> explicitly declare a file system mount should trust its owner/group and
> then that can also be used to wire up any other "unsafe" activities in a
> general "mounted for a special use" option.

I think that a mount option to enable it isn't a bad idea, but the
mount option should be something generic and not be about "this group
is special" which it sounds like google is currently using.

More like "enable hole punching" - which doesn't enable it
unconditionally, you'd still have the security checks.

             Linus

[toc] | [prev] | [next] | [standalone]

#1356092

On Fri, Mar 11, 2016 at 9:23 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Mar 11, 2016 at 5:59 AM, One Thousand Gnomes
> <gnomes@lxorguk.ukuu.org.uk> wrote:
>>
>> > > 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.
>>
>> Not if you don't have access to a block device node to open it, or there
>> are SELinux rules that control the access. There are cases it isn't
>> entirely the same thing as far as I can see. Consider within a container
>> for example.
>
> I agree that it's not the same thing, but I don't think it really ends
> up mattering.
>
> Either the container is properly separated and set up - in which case
> the uid mapping is what protects you - or it isn't - in which case the
> container could just mknod whatever hell node it wants anyway.
>
> So we do pretty much have the permission model.

This makes me nervous.

Suppose I unshare my user namespace, set up very restrictive mounts,
drop caps, seccomp the hell out of myself (but allow literally only
read, write, and ioctl and keep only a single fd to a file on an
ordinary filesystem, which should be safe), and run untrusted code.

Now that code can do this unsafe ioctl simply because its uid or gid
happens to have read access to a device node that isn't even present
in the sandbox.  Ick.

What if we had an ioctl to do these data-leaking operations that took,
as an extra parameter, an fd to the block device node.  They allow
access if the fd points to the right inode and has FMODE_READ (and LSM
checks say it's okay).  Sure, it's awkward, but it's much safer.

--Andy

[toc] | [prev] | [next] | [standalone]

#1356120

On Fri, Mar 11, 2016 at 9:30 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> What if we had an ioctl to do these data-leaking operations that took,
> as an extra parameter, an fd to the block device node.  They allow
> access if the fd points to the right inode and has FMODE_READ (and LSM
> checks say it's okay).  Sure, it's awkward, but it's much safer.

That sounds absolutely horrible.

I'd *much* prefer the suggestion from Alan to simply have a mount-time
option to enable it. That way, you will never get any surprises, and
no "subtle new behavior for somebody who set their system up in a way
that doesn't allow for this".

So you'd have to explicitly say "my setup is ok with hole punching".

                Linus

[toc] | [prev] | [next] | [standalone]

#1356243 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Fri, Mar 11, 2016 at 10:25:30AM -0800, Linus Torvalds wrote:
> On Fri, Mar 11, 2016 at 9:30 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> >
> > What if we had an ioctl to do these data-leaking operations that took,
> > as an extra parameter, an fd to the block device node.  They allow
> > access if the fd points to the right inode and has FMODE_READ (and LSM
> > checks say it's okay).  Sure, it's awkward, but it's much safer.
> 
> That sounds absolutely horrible.
> 
> I'd *much* prefer the suggestion from Alan to simply have a mount-time
> option to enable it. That way, you will never get any surprises, and
> no "subtle new behavior for somebody who set their system up in a way
> that doesn't allow for this".
> 
> So you'd have to explicitly say "my setup is ok with hole punching".

Except it's not hole punching that is the problem. Hoel punching
makes sure that the underlying blocksare removed and so whatever
data is in them cannot be accessed any more. The problem here is
preallocation of unwritten blocks that expose the stale data if the
filesystem skips marking those blocks as unwritten.

And, so, what happens when a file that is preallocated with the
unwritten bit so it exposes stale data then has it's owner/group
changed? Or copied by root/user in privileged group to a different
location that other users can access? Or any of the other vectors
that can result in the stale data being copied/made available to
unprivileged users?

It's all well and good to restrict access to the fallocate() call to
limit who can expose stale data, but it doesn't remove the fact it
is easy for stale data to unintentionally escape the privileged
group once it has been exposed because there is no record of the
fact the file contains uninitialised blocks....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

#1356301 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Sat, Mar 12, 2016 at 09:30:47AM +1100, Dave Chinner wrote:
> It's all well and good to restrict access to the fallocate() call to
> limit who can expose stale data, but it doesn't remove the fact it
> is easy for stale data to unintentionally escape the privileged
> group once it has been exposed because there is no record of the
> fact the file contains uninitialised blocks....

Ultimately, it's up to the trusted process to make sure it never
reveals any stale data.  For example, if you have the policy that all
data is encrypted at rest, and the trusted process is always going to
be decrypting any blocks it reads from disk before passing it on its
client (for example) then any stale data is going to be obscured by
the decryption step before it gets passed on.

At the end of the day it's about whether you trust the userspace
program or not.  I know there's a long and venerated traition of
assuming that all application programmers are incompetent, but that
leads to file systems doing more work than what is strictly necessary,
and that has a performance tax.

And if the result is the cluster file system authors decide to create
a user space file system, and bypass the kernel file system directly,
then we have to trust them to do a competent job anyway.  But if you
believe that there are still ways in which a in-kernel file system can
add value, then it's encumbent on us to to be a bit more flexible and
not assume that all userspace programmers are blithering idiots.

Cheers,

							- Ted

[toc] | [prev] | [next] | [standalone]

#1356303

On Fri, Mar 11, 2016 at 4:35 PM, Theodore Ts'o <tytso@mit.edu> wrote:
>
> At the end of the day it's about whether you trust the userspace
> program or not.

There's a big difference between "give the user rope", and "tie the
rope in a noose and put a banana peel so that the user might stumble
into the rope and hang himself", though.

So I do think that Dave is right that we should also strive to make
sure that our interfaces are not just secure in theory, but that they
are also good interfaces to make mistakes less likely.

I think we _should_ give users rope, but maybe we should also make
sure that there isn't some hidden rapidly spinning saw-blade right
next to the rope that the user doesn't even think about.

                   Linus

[toc] | [prev] | [next] | [standalone]

#1356362 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Fri, Mar 11, 2016 at 04:44:16PM -0800, Linus Torvalds wrote:
> On Fri, Mar 11, 2016 at 4:35 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> >
> > At the end of the day it's about whether you trust the userspace
> > program or not.
> 
> There's a big difference between "give the user rope", and "tie the
> rope in a noose and put a banana peel so that the user might stumble
> into the rope and hang himself", though.

So let's see.  The user application has to explicitly request
NO_HIDE_STALE via an fallocate flag --- so it requires changing the
source code and recompiling the application.  And then, the system
administrator has to pass in a mount option specifying a group that
the application has to run under.  And then the application has to run
setgid with that group's privileges.

I hardly think that can be considered handing the user a pre-tied
noose.

Sure, the application can do something stupid --- but I'd arguing
giving root to some junior sysadmin is far more likely to cause
problems.

Cheers,

						- Ted

[toc] | [prev] | [next] | [standalone]

#1356388 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On 03/12/2016 08:19 AM, Theodore Ts'o wrote:
> On Fri, Mar 11, 2016 at 04:44:16PM -0800, Linus Torvalds wrote:
>>
>> There's a big difference between "give the user rope", and "tie the
>> rope in a noose and put a banana peel so that the user might stumble
>> into the rope and hang himself", though.
> [...]  And then the application has to run
> setgid with that group's privileges.

Your concept of hierarchically nesting containers via filesystem 
instances looks nice to me.

A potential concern could be whether gids are the right implementation 
for expressing hierarchically nested access permissions in a persistent way.

Your permissions attached to gids are nested (because inside of your 
containers you may have another instance of a completely different gid 
namespace), they are also persistent when your mount flags etc are 
restored properly after a crash (by some scripts), but probably use of 
gids for this might look like a kind of "misuse" of the original gid 
concept from the 1970s.

Maybe you currently don't have a better /persistent/ concept for 
expressing your needs, so maybe your solution could be just fine under 
the currently given cirumstances.

Introduction of a new concept for overcoming the current limitations 
must be done very carefully.

The bad discard semantics concerns about information leaks could be 
/hypothetically/ solved at /concept level/ in the following way. Please 
note that by "concept level" I don't want to imply any particular 
implementation, this is just a mental experiment for discussion of the 
problems,  just a "model of thinking":

a) Use a hierarchical namespace for naming subjects, e.g. 
hypervisorA.containerB.subcontainerC.user9 instead of gid=9

b) Attach actual permissions to each block of the underlying block 
device (fine-grained object model).

c) Correctly maintain access rights at each hierarchical layer, and for 
all operations (including discard with whatever semantics). In case some 
inner instance is untrusted and may do evil things, this will be 
intercepted / corrected at outer layers (which are more trusted). In 
essence, the nesting hierarchy is also a hierarchy of trust.

Now information leaks by bad discard semantics etc should be solved at 
any level, even regarding completely unrelated containers or users, as 
long as no physical access to the disk is possible. In addition, 
encryption may be used for even overcoming this.

Of course, a direct implementation of such extremely fine-grained access 
permissions would carry way too much overhead. Both the number of 
subjects as well as the number of objects must be reduced to some 
reasonable order of magnitude, at least at outer levels.

Thus the question is: how can we achieve almost the same effect with 
much less overhead?


Hmm, in my old Athomux research prototype, I proposed some solutions for 
this, on an academic green meadow. But I am unsure what is transferable 
to a standard POSIX semantics system, and what not. Rethinking these 
concepts as well as checking them may take some time....

Here is a first alpha-stage attempt:

1) Give up the hierarchical subject namespace a), but maybe not fully. 
Access checking will continue /locally/ at each layer, by treating each 
subsystem as a (grey) blackbox. This is already the default 
implementation strategy. The total system may be less secure than in an 
idealized fine-grained system, because outer levels can no longer detect 
bad guys inside of their subsystem instances. The question is: how to 
get a "more secure" system than currently, with some reasonable effort.

2) Some /coarse/ access permission checks at the block layer b), but 
finer than today. Currently there is almost no checking at all (except 
when accessing a huge block device as a whole during open() => at 1&1 we 
have very large ones, and they may continue running for years). I am 
unsure how to achieve this in detail.

An idea for a long-term solution would be offloading of "allocation 
groups" to the block layer (if their size is coarsely dynamic in 
general, e.g. in steps of gigabytes), and to implement some coarse 
permission checks there. These could then be related to "containers" or 
"container groups". One of the problems is that some wide-spread network 
protocols like iSCSI have no clue about this, so this can only be an 
optional new feature.

Further ideas sought.

Cheers, Thomas

P.S. The concept of a "nest" in Athomux was already some kind of 
"recursively nested block device".

[toc] | [prev] | [next] | [standalone]

#1356776 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Fri, Mar 11, 2016 at 04:44:16PM -0800, Linus Torvalds wrote:
> On Fri, Mar 11, 2016 at 4:35 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> >
> > At the end of the day it's about whether you trust the userspace
> > program or not.
> 
> There's a big difference between "give the user rope", and "tie the
> rope in a noose and put a banana peel so that the user might stumble
> into the rope and hang himself", though.
> 
> So I do think that Dave is right that we should also strive to make
> sure that our interfaces are not just secure in theory, but that they
> are also good interfaces to make mistakes less likely.

At which point I have to ask: how do we safely allow filesystems to
expose stale data in files? There's a big "we need to trust
userspace" component in ever proposal that has been made so far -
that's the part I have extreme trouble with.

For example, what happens when a backup process running as root a
file that has exposed stale data? Yes, we could set the "NODUMP"
flag on the inode to tell backup programs to skip backing up such
files, but we're now trusting some random userspace application
(e.g. tar, rsync, etc) not to do something we don't want it to do
with the data in that file.

AFAICT, we can't stop root from copying files that have exposed
stale data or changing their ownership without some kind of special
handling of "contains stale data" files within the kernel. At this
point we are back to needing persistent tracking of the "exposed
stale data" state in the inode as the only safe way to allow us to
expose stale data.  That's fairly ironic given that the stated
purpose of exposing stale data through fallocate is to avoid the
overhead of the existing mechanisms we use to track extents
containing stale data....

> I think we _should_ give users rope, but maybe we should also make
> sure that there isn't some hidden rapidly spinning saw-blade right
> next to the rope that the user doesn't even think about.

IMO we already have a good, safe interface that provides the rope
without the saw blades. I'm happy to be proven wrong, but IMO I
don't see that we can provide stale data exposure in a safe,
non-saw-bladey way without any kernel/filesystem side overhead.....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

#1357132 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On 03/13/2016 07:30 PM, Dave Chinner wrote:
> On Fri, Mar 11, 2016 at 04:44:16PM -0800, Linus Torvalds wrote:
>> On Fri, Mar 11, 2016 at 4:35 PM, Theodore Ts'o <tytso@mit.edu> wrote:
>>> At the end of the day it's about whether you trust the userspace
>>> program or not.
>> There's a big difference between "give the user rope", and "tie the
>> rope in a noose and put a banana peel so that the user might stumble
>> into the rope and hang himself", though.
>>
>> So I do think that Dave is right that we should also strive to make
>> sure that our interfaces are not just secure in theory, but that they
>> are also good interfaces to make mistakes less likely.
> At which point I have to ask: how do we safely allow filesystems to
> expose stale data in files? There's a big "we need to trust
> userspace" component in ever proposal that has been made so far -
> that's the part I have extreme trouble with.
>
> For example, what happens when a backup process running as root a
> file that has exposed stale data? Yes, we could set the "NODUMP"
> flag on the inode to tell backup programs to skip backing up such
> files, but we're now trusting some random userspace application
> (e.g. tar, rsync, etc) not to do something we don't want it to do
> with the data in that file.
>
> AFAICT, we can't stop root from copying files that have exposed
> stale data or changing their ownership without some kind of special
> handling of "contains stale data" files within the kernel. At this
> point we are back to needing persistent tracking of the "exposed
> stale data" state in the inode as the only safe way to allow us to
> expose stale data.  That's fairly ironic given that the stated
> purpose of exposing stale data through fallocate is to avoid the
> overhead of the existing mechanisms we use to track extents
> containing stale data....

I think that once we enter this mode, the local file system has effectively 
ceded its role to prevent stale data exposure to the upper layer. In effect, 
this ceases to become a normal file system for any enabled process if we control 
this through fallocate() or for all processes if we do the brute force mount 
option that would be file system wide.

That means we would not need to track this. Extents would be marked as if they 
always have had valid data (no more allocated but unwritten state).

In the end, that is the actual goal - move this enforcement up a layer for 
overlay/user space file systems that are then responsible for policing this ind 
of thing.

Regards,

Ric

>
>> I think we _should_ give users rope, but maybe we should also make
>> sure that there isn't some hidden rapidly spinning saw-blade right
>> next to the rope that the user doesn't even think about.
> IMO we already have a good, safe interface that provides the rope
> without the saw blades. I'm happy to be proven wrong, but IMO I
> don't see that we can provide stale data exposure in a safe,
> non-saw-bladey way without any kernel/filesystem side overhead.....
>
> Cheers,
>
> Dave.

[toc] | [prev] | [next] | [standalone]

#1357308 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Mon, Mar 14, 2016 at 06:34:00AM -0400, Ric Wheeler wrote:
> I think that once we enter this mode, the local file system has effectively
> ceded its role to prevent stale data exposure to the upper layer. In effect,
> this ceases to become a normal file system for any enabled process if we
> control this through fallocate() or for all processes if we do the brute
> force mount option that would be file system wide.

Or we do this via group id, such that we are ceding responsibility for
proventing stale data exposure to the processes running under that
group id.  That process has the responsibility for making sure that it
doesn't return any data from that file unless it has been written, and
also to make sure the permissions of that file are not readable by
processes that aren't in that group.  (For example, owned by user
ceph, group ceph, with premissions 640).

> In the end, that is the actual goal - move this enforcement up a layer for
> overlay/user space file systems that are then responsible for policing this
> ind of thing.

Yes, exactly.

						- Ted

[toc] | [prev] | [next] | [standalone]

#1358224 — Re: [PATCH 2/2] block: create ioctl to discard-or-zeroout a range of blocks

On Mon, Mar 14, 2016 at 10:46:03AM -0400, Theodore Ts'o wrote:
> On Mon, Mar 14, 2016 at 06:34:00AM -0400, Ric Wheeler wrote:
> > I think that once we enter this mode, the local file system has effectively
> > ceded its role to prevent stale data exposure to the upper layer. In effect,
> > this ceases to become a normal file system for any enabled process if we
> > control this through fallocate() or for all processes if we do the brute
> > force mount option that would be file system wide.
> 
> Or we do this via group id, such that we are ceding responsibility for
> proventing stale data exposure to the processes running under that
> group id.  That process has the responsibility for making sure that it
> doesn't return any data from that file unless it has been written, and
> also to make sure the permissions of that file are not readable by
> processes that aren't in that group.  (For example, owned by user
> ceph, group ceph, with premissions 640).

Root can still change the group id of a file that has exposed stale
data and hence make it visible outside of the group based
containment wall. i.e. external actors can still unintentionally
expose stale data, even though the application might be correctly
contained and safe.

What we are missing is actual numbers that show that exposing stale
data is a /significant/ win for these applications that are
demanding it. And then we need evidence proving that the problem is
actually systemic and not just a hack around a bad implementation of
a feature...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web