Re: [PATCH 2/6] ublk_drv: don't probe partitions if the ubq daemon isn't trusted

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022/11/16 14:08, Ming Lei wrote:
> If any ubq daemon is unprivileged, the ublk char device is allowed
> for unprivileged user, and we can't trust the current user, so not
> probe partitions.
> 
> Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
>  drivers/block/ublk_drv.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index fe997848c1ff..a5f3d8330be5 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -149,6 +149,7 @@ struct ublk_device {
>  
>  #define UB_STATE_OPEN		0
>  #define UB_STATE_USED		1
> +#define UB_STATE_PRIVILEGED	2
>  	unsigned long		state;
>  	int			ub_number;
>  
> @@ -161,6 +162,7 @@ struct ublk_device {
>  
>  	struct completion	completion;
>  	unsigned int		nr_queues_ready;
> +	unsigned int		nr_privileged_daemon;
>  
>  	/*
>  	 * Our ubq->daemon may be killed without any notification, so
> @@ -1184,9 +1186,15 @@ static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
>  		ubq->ubq_daemon = current;
>  		get_task_struct(ubq->ubq_daemon);
>  		ub->nr_queues_ready++;
> +
> +		if (capable(CAP_SYS_ADMIN))
> +			ub->nr_privileged_daemon++;
>  	}
> -	if (ub->nr_queues_ready == ub->dev_info.nr_hw_queues)
> +	if (ub->nr_queues_ready == ub->dev_info.nr_hw_queues) {
> +		if (ub->nr_privileged_daemon == ub->nr_queues_ready)

Hi, Ming.

Just like nr_queues_ready, ub->nr_privileged_daemon should be reset
to zero in ublk_ctrl_start_recovery(). otherwise new ubq_daemons are
always treated as unprivileged.

> +			set_bit(UB_STATE_PRIVILEGED, &ub->state);
>  		complete_all(&ub->completion);
> +	}
>  	mutex_unlock(&ub->mutex);
>  }
>  
> @@ -1540,6 +1548,10 @@ static int ublk_ctrl_start_dev(struct io_uring_cmd *cmd)
>  	if (ret)
>  		goto out_put_disk;
>  
> +	/* don't probe partitions if any one ubq daemon is un-trusted */
> +	if (!test_bit(UB_STATE_PRIVILEGED, &ub->state))
> +		set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);

Can we simply check if nr_queues_ready == nr_privileged_daemon here
instead of adding a new bit UB_STATE_PRIVILEGED?

BTW, I think exposing whether ub's state is privileged/unprivileged
to users(./ublk list) is a good idea.

Regards,
Zhang



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux