lkml.org 
[lkml]   [2021]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 2/2] blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
    Date
    On Thu, Jun 03, 2021 at 11:20:33AM +0000, Damien Le Moal wrote:
    > On 2021/06/03 19:07, David Sterba wrote:
    > > On Thu, Jun 03, 2021 at 10:00:08AM +0000, Damien Le Moal wrote:
    > >> On 2021/06/03 18:54, David Sterba wrote:
    > >>> On Mon, May 31, 2021 at 01:54:53PM +0000, Niklas Cassel wrote:
    > >>>> From: Niklas Cassel <niklas.cassel@wdc.com>
    > >>>>
    > >>>> Performing a BLKREPORTZONE operation should be allowed under the same
    > >>>> permissions as read(). (read() does not require CAP_SYS_ADMIN).
    > >>>>
    > >>>> Remove the CAP_SYS_ADMIN requirement, and instead check that the fd was
    > >>>> successfully opened with FMODE_READ. This way BLKREPORTZONE will match
    > >>>> the access control requirement of read().
    > >>>
    > >>> Does this mean that a process that does not have read nor write access
    > >>> to the device itself (blocks) is capable of reading the zone
    > >>> information? Eg. some monitoring tool.
    > >>
    > >> With this change, to do a report zones, the process will only need to have read
    > >> access to the device. And if it has read access, it also means that it can read
    > >> the zones content.
    > >
    > > Ok, so this is a bit restricting. The zone information is like block
    > > device metadata, comparing it to a file that has permissionx 0600 I can
    > > see the all the stat info (name, tiemstamps) but can't read the data.
    > >
    > > But as the ioctl work, it needs a file descriptor and there's probably
    > > no way to separate the permissions to read blocks and just the metadata.
    > > For a monitoring/reporting tool this would be useful. Eg. for btrfs it
    > > could be part of filesystem status overview regarding full or near-full
    > > zones and emitting an early warning or poking some service to start the
    > > reclaim.
    >
    > You lost me... the change is less restrictive than before because the process
    > does not need SYS_CAP_ADMIN anymore. The block device file open is untouched, no
    > change. So whatever process could open it before, will still be able to do so as
    > is. More processes will be able to do report zones with the change. That is all
    > really that changes, so I do not see what potentially breaks, nor how this may
    > prevent writing some monitoring tool. Whoever can open the block device file has
    > FMODE_READ rights, no ? Am I missing something here ?

    What David is saying is that for e.g. stat(), you can get metadata when you
    don't even have read permission for a device, since stat() takes a pathname.

    An ioctl requires you to first do an open(), which will will check permissions,
    so implementing the same is not really possible for an ioctl like BLKREPORTZONE.

    However, I think the current ioctl is fine.
    The amount of data that is transferred from a zoned block device for the zone
    report is more than the data that is transferred when someone does a stat(),
    so in one way getting the zone report is more like a read.

    Doing what David suggests would, as far as I can tell, require another solution
    than the existing ioctl method, which this patch changes.

    We can think about his suggestion, but it would need to be addressed is a
    separate patch series. (If his suggestion is something that we want to pursue.)


    Kind regards,
    Niklas

    \
     
     \ /
      Last update: 2021-06-03 14:01    [W:4.180 / U:3.164 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site