Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: iAWUoYi/cPNikzvVoxL6cy441Bq+jISv8D5DCAq3oIcRCwqKU4FFm95Bc6zmKTPl3Qfpa9pcuf
 PKKAh3buVrG5pCoJHVXSYCWcBu/m7O26j04RTFmn1/I2FBrKPm7C/iKcG+EspghW4Ix1QkqSwB
 +ds9axfC7N9BZsErKD1QupqX4Wr9H1C361djdq2D05lk5zisEKxCvFAI4jymarSmx7u8Boxy8Q
 3yUef1TpY0i22t8T4pN7BF4W6EdbwXq3FCE6JDDqfIu/TFESjsA6+D6NqWtBkxWojAzxZI3qZW
 dKQ=
From:   Niklas Cassel <Niklas.Cassel@wdc.com>
To:     Damien Le Moal <Damien.LeMoal@wdc.com>
CC:     "dsterba@suse.cz" <dsterba@suse.cz>, Jens Axboe <axboe@kernel.dk>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
Thread-Topic: [PATCH 2/2] blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
Thread-Index: AQHXViSH/N9K56jFnkyIxAVcrCvP9asCM6IA
Date:   Thu, 3 Jun 2021 11:59:53 +0000
Message-ID: <YLjEOMaxk8/8GOex@x1-carbon>
References: <20210531135444.122018-1-Niklas.Cassel@wdc.com>
 <20210531135444.122018-3-Niklas.Cassel@wdc.com>
 <20210603095117.GU31483@twin.jikos.cz>
 <DM6PR04MB7081B69E31BB7ADDF02E2D9BE73C9@DM6PR04MB7081.namprd04.prod.outlook.com>
 <20210603100436.GV31483@twin.jikos.cz>
 <DM6PR04MB708127C72BDC03B446997DACE73C9@DM6PR04MB7081.namprd04.prod.outlook.com>
In-Reply-To: <DM6PR04MB708127C72BDC03B446997DACE73C9@DM6PR04MB7081.namprd04.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-ID: <860A9E52A96BE54C8AA69B4BC30A70DE@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: a021abab-9796-4783-86bd-08d926871935
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jun 2021 11:59:53.3640
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: h3+bAl47gS1bHuv5WyxNMY1jY13MOJ1bCX0yr9EjaIX7rQhRqcOGl1nXTHkB/hSz1hYkCGhlFlam4MW8C4wj+w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR04MB7733
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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 sam=
e
> >>>> 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 mat=
ch
> >>>> the access control requirement of read().
> >>>
> >>> Does this mean that a process that does not have read nor write acces=
s
> >>> 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.
> >=20
> > 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.
> >=20
> > 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.
>=20
> You lost me... the change is less restrictive than before because the pro=
cess
> does not need SYS_CAP_ADMIN anymore. The block device file open is untouc=
hed, no
> change. So whatever process could open it before, will still be able to d=
o 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 thi=
s may
> prevent writing some monitoring tool. Whoever can open the block device f=
ile 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 permissi=
ons,
so implementing the same is not really possible for an ioctl like BLKREPORT=
ZONE.

However, I think the current ioctl is fine.
The amount of data that is transferred from a zoned block device for the zo=
ne
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 solu=
tion
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 purs=
ue.)


Kind regards,
Niklas