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