linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN
@ 2021-06-14 12:23 Niklas Cassel
  2021-06-14 12:23 ` [PATCH v3 1/2] blk-zoned: allow zone management send operations " Niklas Cassel
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Niklas Cassel @ 2021-06-14 12:23 UTC (permalink / raw)
  To: Jens Axboe, Shaun Tancheff, Hannes Reinecke, Martin K. Petersen,
	Damien Le Moal
  Cc: Damien Le Moal, Niklas Cassel, Jens Axboe, linux-block, linux-kernel

From: Niklas Cassel <niklas.cassel@wdc.com>

Allow the following blk-zoned ioctls: BLKREPORTZONE, BLKRESETZONE,
BLKOPENZONE, BLKCLOSEZONE, and BLKFINISHZONE to be performed without
CAP_SYS_ADMIN.

Neither read() nor write() requires CAP_SYS_ADMIN, and considering
the close relationship between read()/write() and these ioctls, there
is no reason to require CAP_SYS_ADMIN for these ioctls either.

Changes since v2:
-Drop the FMODE_READ check from patch 2/2.
Right now it is possible to open() the device with O_WRONLY
and get the zone report from that fd. Therefore adding a
FMODE_READ check on BLKREPORTZONE would break existing applications.
Instead, just remove the existing CAP_SYS_ADMIN check.


Niklas Cassel (2):
  blk-zoned: allow zone management send operations without CAP_SYS_ADMIN
  blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN

 block/blk-zoned.c | 6 ------
 1 file changed, 6 deletions(-)

-- 
2.31.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/2] blk-zoned: allow zone management send operations without CAP_SYS_ADMIN
  2021-06-14 12:23 [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN Niklas Cassel
@ 2021-06-14 12:23 ` Niklas Cassel
  2021-06-16 13:42   ` Aravind Ramesh
                     ` (2 more replies)
  2021-06-14 12:23 ` [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE " Niklas Cassel
  2021-06-28  7:20 ` [PATCH v3 0/2] allow blk-zoned ioctls " Niklas Cassel
  2 siblings, 3 replies; 13+ messages in thread
From: Niklas Cassel @ 2021-06-14 12:23 UTC (permalink / raw)
  To: Jens Axboe, Damien Le Moal, Shaun Tancheff, Martin K. Petersen,
	Hannes Reinecke
  Cc: Damien Le Moal, Niklas Cassel, stable, Jens Axboe, linux-block,
	linux-kernel

From: Niklas Cassel <niklas.cassel@wdc.com>

Zone management send operations (BLKRESETZONE, BLKOPENZONE, BLKCLOSEZONE
and BLKFINISHZONE) should be allowed under the same permissions as write().
(write() does not require CAP_SYS_ADMIN).

Additionally, other ioctls like BLKSECDISCARD and BLKZEROOUT only check if
the fd was successfully opened with FMODE_WRITE.
(They do not require CAP_SYS_ADMIN).

Currently, zone management send operations require both CAP_SYS_ADMIN
and that the fd was successfully opened with FMODE_WRITE.

Remove the CAP_SYS_ADMIN requirement, so that zone management send
operations match the access control requirement of write(), BLKSECDISCARD
and BLKZEROOUT.

Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
Cc: stable@vger.kernel.org # v4.10+
---
Changes since v2:
-None

Note to backporter:
Function was added as blkdev_reset_zones_ioctl() in v4.10.
Function was renamed to blkdev_zone_mgmt_ioctl() in v5.5.
The patch is valid both before and after the function rename.

 block/blk-zoned.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 250cb76ee615..0789e6e9f7db 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -349,9 +349,6 @@ int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode,
 	if (!blk_queue_is_zoned(q))
 		return -ENOTTY;
 
-	if (!capable(CAP_SYS_ADMIN))
-		return -EACCES;
-
 	if (!(mode & FMODE_WRITE))
 		return -EBADF;
 
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
  2021-06-14 12:23 [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN Niklas Cassel
  2021-06-14 12:23 ` [PATCH v3 1/2] blk-zoned: allow zone management send operations " Niklas Cassel
@ 2021-06-14 12:23 ` Niklas Cassel
  2021-06-16  2:29   ` Damien Le Moal
                     ` (3 more replies)
  2021-06-28  7:20 ` [PATCH v3 0/2] allow blk-zoned ioctls " Niklas Cassel
  2 siblings, 4 replies; 13+ messages in thread
From: Niklas Cassel @ 2021-06-14 12:23 UTC (permalink / raw)
  To: Jens Axboe, Hannes Reinecke, Martin K. Petersen, Damien Le Moal,
	Shaun Tancheff
  Cc: Damien Le Moal, Niklas Cassel, stable, Jens Axboe, linux-block,
	linux-kernel

From: Niklas Cassel <niklas.cassel@wdc.com>

A user space process should not need the CAP_SYS_ADMIN capability set
in order to perform a BLKREPORTZONE ioctl.

Getting the zone report is required in order to get the write pointer.
Neither read() nor write() requires CAP_SYS_ADMIN, so it is reasonable
that a user space process that can read/write from/to the device, also
can get the write pointer. (Since e.g. writes have to be at the write
pointer.)

Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Cc: stable@vger.kernel.org # v4.10+
---
Changes since v2:
-Drop the FMODE_READ check. Right now it is possible to open() the device with
O_WRONLY and get the zone report from that fd. Therefore adding a FMODE_READ
check on BLKREPORTZONE would break existing applications. Instead, just remove
the existing CAP_SYS_ADMIN check.

 block/blk-zoned.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 0789e6e9f7db..457eceabed2e 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -288,9 +288,6 @@ int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
 	if (!blk_queue_is_zoned(q))
 		return -ENOTTY;
 
-	if (!capable(CAP_SYS_ADMIN))
-		return -EACCES;
-
 	if (copy_from_user(&rep, argp, sizeof(struct blk_zone_report)))
 		return -EFAULT;
 
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
  2021-06-14 12:23 ` [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE " Niklas Cassel
@ 2021-06-16  2:29   ` Damien Le Moal
  2021-06-16 13:43   ` Aravind Ramesh
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-06-16  2:29 UTC (permalink / raw)
  To: Niklas Cassel, Jens Axboe, Hannes Reinecke, Martin K. Petersen,
	Shaun Tancheff
  Cc: stable, Jens Axboe, linux-block, linux-kernel

On 2021/06/14 21:23, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> A user space process should not need the CAP_SYS_ADMIN capability set
> in order to perform a BLKREPORTZONE ioctl.
> 
> Getting the zone report is required in order to get the write pointer.
> Neither read() nor write() requires CAP_SYS_ADMIN, so it is reasonable
> that a user space process that can read/write from/to the device, also
> can get the write pointer. (Since e.g. writes have to be at the write
> pointer.)
> 
> Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Cc: stable@vger.kernel.org # v4.10+
> ---
> Changes since v2:
> -Drop the FMODE_READ check. Right now it is possible to open() the device with
> O_WRONLY and get the zone report from that fd. Therefore adding a FMODE_READ
> check on BLKREPORTZONE would break existing applications. Instead, just remove
> the existing CAP_SYS_ADMIN check.
> 
>  block/blk-zoned.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 0789e6e9f7db..457eceabed2e 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -288,9 +288,6 @@ int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
>  	if (!blk_queue_is_zoned(q))
>  		return -ENOTTY;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> -		return -EACCES;
> -
>  	if (copy_from_user(&rep, argp, sizeof(struct blk_zone_report)))
>  		return -EFAULT;
>  
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@dc.com>

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v3 1/2] blk-zoned: allow zone management send operations without CAP_SYS_ADMIN
  2021-06-14 12:23 ` [PATCH v3 1/2] blk-zoned: allow zone management send operations " Niklas Cassel
@ 2021-06-16 13:42   ` Aravind Ramesh
  2021-06-18 14:48   ` Adam Manzanares
  2021-06-18 17:56   ` Himanshu Madhani
  2 siblings, 0 replies; 13+ messages in thread
From: Aravind Ramesh @ 2021-06-16 13:42 UTC (permalink / raw)
  To: Niklas Cassel, Jens Axboe, Damien Le Moal, Shaun Tancheff,
	Martin K. Petersen, Hannes Reinecke
  Cc: Damien Le Moal, Niklas Cassel, stable, Jens Axboe, linux-block,
	linux-kernel



> -----Original Message-----
> From: Niklas Cassel <Niklas.Cassel@wdc.com>
> Sent: Monday, June 14, 2021 5:53 PM
> To: Jens Axboe <axboe@kernel.dk>; Damien Le Moal <Damien.LeMoal@wdc.com>;
> Shaun Tancheff <shaun@tancheff.com>; Martin K. Petersen
> <martin.petersen@oracle.com>; Hannes Reinecke <hare@suse.com>
> Cc: Damien Le Moal <Damien.LeMoal@wdc.com>; Niklas Cassel
> <Niklas.Cassel@wdc.com>; stable@vger.kernel.org; Jens Axboe <axboe@fb.com>;
> linux-block@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH v3 1/2] blk-zoned: allow zone management send operations
> without CAP_SYS_ADMIN
> 
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Zone management send operations (BLKRESETZONE, BLKOPENZONE,
> BLKCLOSEZONE and BLKFINISHZONE) should be allowed under the same permissions
> as write().
> (write() does not require CAP_SYS_ADMIN).
> 
> Additionally, other ioctls like BLKSECDISCARD and BLKZEROOUT only check if the fd
> was successfully opened with FMODE_WRITE.
> (They do not require CAP_SYS_ADMIN).
> 
> Currently, zone management send operations require both CAP_SYS_ADMIN and
> that the fd was successfully opened with FMODE_WRITE.
> 
> Remove the CAP_SYS_ADMIN requirement, so that zone management send
> operations match the access control requirement of write(), BLKSECDISCARD and
> BLKZEROOUT.
> 
> Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> Cc: stable@vger.kernel.org # v4.10+
> ---
> Changes since v2:
> -None
> 
> Note to backporter:
> Function was added as blkdev_reset_zones_ioctl() in v4.10.
> Function was renamed to blkdev_zone_mgmt_ioctl() in v5.5.
> The patch is valid both before and after the function rename.
> 
>  block/blk-zoned.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c index
> 250cb76ee615..0789e6e9f7db 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -349,9 +349,6 @@ int blkdev_zone_mgmt_ioctl(struct block_device *bdev,
> fmode_t mode,
>  	if (!blk_queue_is_zoned(q))
>  		return -ENOTTY;
> 
> -	if (!capable(CAP_SYS_ADMIN))
> -		return -EACCES;
> -
>  	if (!(mode & FMODE_WRITE))
>  		return -EBADF;
> 
> --
> 2.31.1

Looks good,

Reviewed-by: Aravind Ramesh <aravind.ramesh@wdc.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
  2021-06-14 12:23 ` [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE " Niklas Cassel
  2021-06-16  2:29   ` Damien Le Moal
@ 2021-06-16 13:43   ` Aravind Ramesh
  2021-06-18 14:49   ` Adam Manzanares
  2021-06-18 17:57   ` Himanshu Madhani
  3 siblings, 0 replies; 13+ messages in thread
From: Aravind Ramesh @ 2021-06-16 13:43 UTC (permalink / raw)
  To: Niklas Cassel, Jens Axboe, Hannes Reinecke, Martin K. Petersen,
	Damien Le Moal, Shaun Tancheff
  Cc: Damien Le Moal, Niklas Cassel, stable, Jens Axboe, linux-block,
	linux-kernel



> -----Original Message-----
> From: Niklas Cassel <Niklas.Cassel@wdc.com>
> Sent: Monday, June 14, 2021 5:53 PM
> To: Jens Axboe <axboe@kernel.dk>; Hannes Reinecke <hare@suse.com>; Martin K.
> Petersen <martin.petersen@oracle.com>; Damien Le Moal
> <Damien.LeMoal@wdc.com>; Shaun Tancheff <shaun@tancheff.com>
> Cc: Damien Le Moal <Damien.LeMoal@wdc.com>; Niklas Cassel
> <Niklas.Cassel@wdc.com>; stable@vger.kernel.org; Jens Axboe <axboe@fb.com>;
> linux-block@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE without
> CAP_SYS_ADMIN
> 
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> A user space process should not need the CAP_SYS_ADMIN capability set in order to
> perform a BLKREPORTZONE ioctl.
> 
> Getting the zone report is required in order to get the write pointer.
> Neither read() nor write() requires CAP_SYS_ADMIN, so it is reasonable that a user
> space process that can read/write from/to the device, also can get the write pointer.
> (Since e.g. writes have to be at the write
> pointer.)
> 
> Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Cc: stable@vger.kernel.org # v4.10+
> ---
> Changes since v2:
> -Drop the FMODE_READ check. Right now it is possible to open() the device with
> O_WRONLY and get the zone report from that fd. Therefore adding a FMODE_READ
> check on BLKREPORTZONE would break existing applications. Instead, just remove
> the existing CAP_SYS_ADMIN check.
> 
>  block/blk-zoned.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c index
> 0789e6e9f7db..457eceabed2e 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -288,9 +288,6 @@ int blkdev_report_zones_ioctl(struct block_device *bdev,
> fmode_t mode,
>  	if (!blk_queue_is_zoned(q))
>  		return -ENOTTY;
> 
> -	if (!capable(CAP_SYS_ADMIN))
> -		return -EACCES;
> -
>  	if (copy_from_user(&rep, argp, sizeof(struct blk_zone_report)))
>  		return -EFAULT;
> 
> --
> 2.31.1

Looks good,

Reviewed-by: Aravind Ramesh <aravind.ramesh@wdc.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/2] blk-zoned: allow zone management send operations without CAP_SYS_ADMIN
  2021-06-14 12:23 ` [PATCH v3 1/2] blk-zoned: allow zone management send operations " Niklas Cassel
  2021-06-16 13:42   ` Aravind Ramesh
@ 2021-06-18 14:48   ` Adam Manzanares
  2021-06-18 17:56   ` Himanshu Madhani
  2 siblings, 0 replies; 13+ messages in thread
From: Adam Manzanares @ 2021-06-18 14:48 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Jens Axboe, Damien Le Moal, Shaun Tancheff, Martin K. Petersen,
	Hannes Reinecke, stable, Jens Axboe, linux-block, linux-kernel

On Mon, Jun 14, 2021 at 12:23:20PM +0000, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Zone management send operations (BLKRESETZONE, BLKOPENZONE, BLKCLOSEZONE
> and BLKFINISHZONE) should be allowed under the same permissions as write().
> (write() does not require CAP_SYS_ADMIN).
> 
> Additionally, other ioctls like BLKSECDISCARD and BLKZEROOUT only check if
> the fd was successfully opened with FMODE_WRITE.
> (They do not require CAP_SYS_ADMIN).
> 
> Currently, zone management send operations require both CAP_SYS_ADMIN
> and that the fd was successfully opened with FMODE_WRITE.
> 
> Remove the CAP_SYS_ADMIN requirement, so that zone management send
> operations match the access control requirement of write(), BLKSECDISCARD
> and BLKZEROOUT.
> 
> Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> Cc: stable@vger.kernel.org # v4.10+
> ---
> Changes since v2:
> -None
> 
> Note to backporter:
> Function was added as blkdev_reset_zones_ioctl() in v4.10.
> Function was renamed to blkdev_zone_mgmt_ioctl() in v5.5.
> The patch is valid both before and after the function rename.
> 
>  block/blk-zoned.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 250cb76ee615..0789e6e9f7db 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -349,9 +349,6 @@ int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode,
>  	if (!blk_queue_is_zoned(q))
>  		return -ENOTTY;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> -		return -EACCES;
> -
>  	if (!(mode & FMODE_WRITE))
>  		return -EBADF;
>  
> -- 
> 2.31.1

LGTM

Reviewed-by: Adam Manzanares <a.manzanares@samsung.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
  2021-06-14 12:23 ` [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE " Niklas Cassel
  2021-06-16  2:29   ` Damien Le Moal
  2021-06-16 13:43   ` Aravind Ramesh
@ 2021-06-18 14:49   ` Adam Manzanares
  2021-06-18 17:57   ` Himanshu Madhani
  3 siblings, 0 replies; 13+ messages in thread
From: Adam Manzanares @ 2021-06-18 14:49 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Jens Axboe, Hannes Reinecke, Martin K. Petersen, Damien Le Moal,
	Shaun Tancheff, stable, Jens Axboe, linux-block, linux-kernel

On Mon, Jun 14, 2021 at 12:23:21PM +0000, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> A user space process should not need the CAP_SYS_ADMIN capability set
> in order to perform a BLKREPORTZONE ioctl.
> 
> Getting the zone report is required in order to get the write pointer.
> Neither read() nor write() requires CAP_SYS_ADMIN, so it is reasonable
> that a user space process that can read/write from/to the device, also
> can get the write pointer. (Since e.g. writes have to be at the write
> pointer.)
> 
> Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Cc: stable@vger.kernel.org # v4.10+
> ---
> Changes since v2:
> -Drop the FMODE_READ check. Right now it is possible to open() the device with
> O_WRONLY and get the zone report from that fd. Therefore adding a FMODE_READ
> check on BLKREPORTZONE would break existing applications. Instead, just remove
> the existing CAP_SYS_ADMIN check.
> 
>  block/blk-zoned.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 0789e6e9f7db..457eceabed2e 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -288,9 +288,6 @@ int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
>  	if (!blk_queue_is_zoned(q))
>  		return -ENOTTY;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> -		return -EACCES;
> -
>  	if (copy_from_user(&rep, argp, sizeof(struct blk_zone_report)))
>  		return -EFAULT;
>  
> -- 
> 2.31.1

LGTM

Reviewed-by: Adam Manzanares <a.manzanares@samsung.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/2] blk-zoned: allow zone management send operations without CAP_SYS_ADMIN
  2021-06-14 12:23 ` [PATCH v3 1/2] blk-zoned: allow zone management send operations " Niklas Cassel
  2021-06-16 13:42   ` Aravind Ramesh
  2021-06-18 14:48   ` Adam Manzanares
@ 2021-06-18 17:56   ` Himanshu Madhani
  2 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2021-06-18 17:56 UTC (permalink / raw)
  To: Niklas Cassel, Jens Axboe, Damien Le Moal, Shaun Tancheff,
	Martin K. Petersen, Hannes Reinecke
  Cc: stable, Jens Axboe, linux-block, linux-kernel



On 6/14/21 7:23 AM, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Zone management send operations (BLKRESETZONE, BLKOPENZONE, BLKCLOSEZONE
> and BLKFINISHZONE) should be allowed under the same permissions as write().
> (write() does not require CAP_SYS_ADMIN).
> 
> Additionally, other ioctls like BLKSECDISCARD and BLKZEROOUT only check if
> the fd was successfully opened with FMODE_WRITE.
> (They do not require CAP_SYS_ADMIN).
> 
> Currently, zone management send operations require both CAP_SYS_ADMIN
> and that the fd was successfully opened with FMODE_WRITE.
> 
> Remove the CAP_SYS_ADMIN requirement, so that zone management send
> operations match the access control requirement of write(), BLKSECDISCARD
> and BLKZEROOUT.
> 
> Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> Cc: stable@vger.kernel.org # v4.10+
> ---
> Changes since v2:
> -None
> 
> Note to backporter:
> Function was added as blkdev_reset_zones_ioctl() in v4.10.
> Function was renamed to blkdev_zone_mgmt_ioctl() in v5.5.
> The patch is valid both before and after the function rename.
> 
>   block/blk-zoned.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 250cb76ee615..0789e6e9f7db 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -349,9 +349,6 @@ int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode,
>   	if (!blk_queue_is_zoned(q))
>   		return -ENOTTY;
>   
> -	if (!capable(CAP_SYS_ADMIN))
> -		return -EACCES;
> -
>   	if (!(mode & FMODE_WRITE))
>   		return -EBADF;
>   
> 

Looks Good.

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani                                Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
  2021-06-14 12:23 ` [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE " Niklas Cassel
                     ` (2 preceding siblings ...)
  2021-06-18 14:49   ` Adam Manzanares
@ 2021-06-18 17:57   ` Himanshu Madhani
  3 siblings, 0 replies; 13+ messages in thread
From: Himanshu Madhani @ 2021-06-18 17:57 UTC (permalink / raw)
  To: Niklas Cassel, Jens Axboe, Hannes Reinecke, Martin K. Petersen,
	Damien Le Moal, Shaun Tancheff
  Cc: stable, Jens Axboe, linux-block, linux-kernel



On 6/14/21 7:23 AM, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> A user space process should not need the CAP_SYS_ADMIN capability set
> in order to perform a BLKREPORTZONE ioctl.
> 
> Getting the zone report is required in order to get the write pointer.
> Neither read() nor write() requires CAP_SYS_ADMIN, so it is reasonable
> that a user space process that can read/write from/to the device, also
> can get the write pointer. (Since e.g. writes have to be at the write
> pointer.)
> 
> Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls")
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Cc: stable@vger.kernel.org # v4.10+
> ---
> Changes since v2:
> -Drop the FMODE_READ check. Right now it is possible to open() the device with
> O_WRONLY and get the zone report from that fd. Therefore adding a FMODE_READ
> check on BLKREPORTZONE would break existing applications. Instead, just remove
> the existing CAP_SYS_ADMIN check.
> 
>   block/blk-zoned.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 0789e6e9f7db..457eceabed2e 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -288,9 +288,6 @@ int blkdev_report_zones_ioctl(struct block_device *bdev, fmode_t mode,
>   	if (!blk_queue_is_zoned(q))
>   		return -ENOTTY;
>   
> -	if (!capable(CAP_SYS_ADMIN))
> -		return -EACCES;
> -
>   	if (copy_from_user(&rep, argp, sizeof(struct blk_zone_report)))
>   		return -EFAULT;
>   
> 

Looks Good.

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
-- 
Himanshu Madhani                                Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN
  2021-06-14 12:23 [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN Niklas Cassel
  2021-06-14 12:23 ` [PATCH v3 1/2] blk-zoned: allow zone management send operations " Niklas Cassel
  2021-06-14 12:23 ` [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE " Niklas Cassel
@ 2021-06-28  7:20 ` Niklas Cassel
  2021-07-05 11:26   ` Niklas Cassel
  2 siblings, 1 reply; 13+ messages in thread
From: Niklas Cassel @ 2021-06-28  7:20 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Jens Axboe, linux-block, linux-kernel

On Mon, Jun 14, 2021 at 12:23:19PM +0000, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Allow the following blk-zoned ioctls: BLKREPORTZONE, BLKRESETZONE,
> BLKOPENZONE, BLKCLOSEZONE, and BLKFINISHZONE to be performed without
> CAP_SYS_ADMIN.
> 
> Neither read() nor write() requires CAP_SYS_ADMIN, and considering
> the close relationship between read()/write() and these ioctls, there
> is no reason to require CAP_SYS_ADMIN for these ioctls either.
> 
> Changes since v2:
> -Drop the FMODE_READ check from patch 2/2.
> Right now it is possible to open() the device with O_WRONLY
> and get the zone report from that fd. Therefore adding a
> FMODE_READ check on BLKREPORTZONE would break existing applications.
> Instead, just remove the existing CAP_SYS_ADMIN check.
> 
> 
> Niklas Cassel (2):
>   blk-zoned: allow zone management send operations without CAP_SYS_ADMIN
>   blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
> 
>  block/blk-zoned.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> -- 
> 2.31.1

Hello Jens,


A gentle ping on this series.

I think it has sufficient Reviewed-by tags by now.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN
  2021-06-28  7:20 ` [PATCH v3 0/2] allow blk-zoned ioctls " Niklas Cassel
@ 2021-07-05 11:26   ` Niklas Cassel
  2021-07-21  5:04     ` Aravind Ramesh
  0 siblings, 1 reply; 13+ messages in thread
From: Niklas Cassel @ 2021-07-05 11:26 UTC (permalink / raw)
  To: Jens Axboe, Jens Axboe; +Cc: linux-block, linux-kernel

On Mon, Jun 28, 2021 at 09:20:15AM +0200, Niklas Cassel wrote:
> On Mon, Jun 14, 2021 at 12:23:19PM +0000, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > 
> > Allow the following blk-zoned ioctls: BLKREPORTZONE, BLKRESETZONE,
> > BLKOPENZONE, BLKCLOSEZONE, and BLKFINISHZONE to be performed without
> > CAP_SYS_ADMIN.
> > 
> > Neither read() nor write() requires CAP_SYS_ADMIN, and considering
> > the close relationship between read()/write() and these ioctls, there
> > is no reason to require CAP_SYS_ADMIN for these ioctls either.
> > 
> > Changes since v2:
> > -Drop the FMODE_READ check from patch 2/2.
> > Right now it is possible to open() the device with O_WRONLY
> > and get the zone report from that fd. Therefore adding a
> > FMODE_READ check on BLKREPORTZONE would break existing applications.
> > Instead, just remove the existing CAP_SYS_ADMIN check.
> > 
> > 
> > Niklas Cassel (2):
> >   blk-zoned: allow zone management send operations without CAP_SYS_ADMIN
> >   blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
> > 
> >  block/blk-zoned.c | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > -- 
> > 2.31.1
> 
> Hello Jens,
> 
> 
> A gentle ping on this series.
> 
> I think it has sufficient Reviewed-by tags by now.
> 
> 
> Kind regards,
> Niklas

Hello again Jens,


any chance of this series being picked up?


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN
  2021-07-05 11:26   ` Niklas Cassel
@ 2021-07-21  5:04     ` Aravind Ramesh
  0 siblings, 0 replies; 13+ messages in thread
From: Aravind Ramesh @ 2021-07-21  5:04 UTC (permalink / raw)
  To: Jens Axboe, Jens Axboe; +Cc: linux-block, linux-kernel, Niklas Cassel


> -----Original Message-----
> From: Niklas Cassel <Niklas.Cassel@wdc.com>
> Sent: Monday, July 5, 2021 4:57 PM
> To: Jens Axboe <axboe@kernel.dk>; Jens Axboe <axboe@fb.com>
> Cc: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN
> 
> On Mon, Jun 28, 2021 at 09:20:15AM +0200, Niklas Cassel wrote:
> > On Mon, Jun 14, 2021 at 12:23:19PM +0000, Niklas Cassel wrote:
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > >
> > > Allow the following blk-zoned ioctls: BLKREPORTZONE, BLKRESETZONE,
> > > BLKOPENZONE, BLKCLOSEZONE, and BLKFINISHZONE to be performed without
> > > CAP_SYS_ADMIN.
> > >
> > > Neither read() nor write() requires CAP_SYS_ADMIN, and considering
> > > the close relationship between read()/write() and these ioctls,
> > > there is no reason to require CAP_SYS_ADMIN for these ioctls either.
> > >
> > > Changes since v2:
> > > -Drop the FMODE_READ check from patch 2/2.
> > > Right now it is possible to open() the device with O_WRONLY and get
> > > the zone report from that fd. Therefore adding a FMODE_READ check on
> > > BLKREPORTZONE would break existing applications.
> > > Instead, just remove the existing CAP_SYS_ADMIN check.
> > >
> > >
> > > Niklas Cassel (2):
> > >   blk-zoned: allow zone management send operations without CAP_SYS_ADMIN
> > >   blk-zoned: allow BLKREPORTZONE without CAP_SYS_ADMIN
> > >
> > >  block/blk-zoned.c | 6 ------
> > >  1 file changed, 6 deletions(-)
> > >
> > > --
> > > 2.31.1
> >
> > Hello Jens,
> >
> >
> > A gentle ping on this series.
> >
> > I think it has sufficient Reviewed-by tags by now.
> >
> >
> > Kind regards,
> > Niklas
> 
> Hello again Jens,
> 
> 
> any chance of this series being picked up?
> 
Hello Jens,

Gentle ping.
Could you please take a look at this series ?

Thanks,
Aravind

> 
> Kind regards,
> Niklas

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-07-21  5:04 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-14 12:23 [PATCH v3 0/2] allow blk-zoned ioctls without CAP_SYS_ADMIN Niklas Cassel
2021-06-14 12:23 ` [PATCH v3 1/2] blk-zoned: allow zone management send operations " Niklas Cassel
2021-06-16 13:42   ` Aravind Ramesh
2021-06-18 14:48   ` Adam Manzanares
2021-06-18 17:56   ` Himanshu Madhani
2021-06-14 12:23 ` [PATCH v3 2/2] blk-zoned: allow BLKREPORTZONE " Niklas Cassel
2021-06-16  2:29   ` Damien Le Moal
2021-06-16 13:43   ` Aravind Ramesh
2021-06-18 14:49   ` Adam Manzanares
2021-06-18 17:57   ` Himanshu Madhani
2021-06-28  7:20 ` [PATCH v3 0/2] allow blk-zoned ioctls " Niklas Cassel
2021-07-05 11:26   ` Niklas Cassel
2021-07-21  5:04     ` Aravind Ramesh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).