Re: [PATCH v3] dm: don't allow ioctls to targets that don't map to whole devices

[Johannes Thumshirn]

On 02/03/2017 10:41 PM, Bart Van Assche wrote:

On Fri, 2017-02-03 at 19:17 +0100, Johannes Thumshirn wrote:

Forgotten git add? git commit --amend without git add is such a classic
mistake on my side as well :-/

Are you familiar with the -a option of git commit? Just run

git commit -a --amend
Yes I try to train my muscle memory to do so. But this has drawbacks as 
well,

like adding unrelated files you've edited and then amend them.

Byte,
Johannes

--
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v3] dm: don't allow ioctls to targets that don't map to whole devices

[Bart Van Assche]

On Fri, 2017-02-03 at 19:17 +0100, Johannes Thumshirn wrote:
> Forgotten git add? git commit --amend without git add is such a classic 
> mistake on my side as well :-/

Are you familiar with the -a option of git commit? Just run

git commit -a --amend

Bart.

Re: [PATCH v3] dm: don't allow ioctls to targets that don't map to whole devices

[Johannes Thumshirn]

On 02/03/2017 05:41 PM, Christoph Hellwig wrote:

On Fri, Feb 03, 2017 at 11:39:22AM -0500, Mike Snitzer wrote:

I assume you meant for v3 to remove the newline? ;)

I did.  And I swear I did edit the file, but I guess the ammend
didn't work.  I guess it's time for the weekend..  I'll resend after
I got some rest.


Forgotten git add? git commit --amend without git add is such a classic 
mistake on my side as well :-/



Anyways,

Reviewed-by: Johannes Thumshirn 

--
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v3] dm: don't allow ioctls to targets that don't map to whole devices

[Paolo Bonzini]

On 03/02/2017 08:37, Christoph Hellwig wrote:
> .. at least for unprivileged users.  Before we called into the SCSI
> ioctl code to allow excemptions for a few SCSI passthrough ioctls,
> but this is pretty unsafe and except for this call dm knows nothing
> about SCSI ioctls.
> 
> As the SCSI ioctl code is now optional, we really don't want to
> drag it in for DM, and the exception is not very useful anyway.
> 
> Signed-off-by: Christoph Hellwig 
> Acked-by: Mike Snitzer 
> ---
>  drivers/md/dm.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 9e958bc94fed..fd4331aa2e19 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -465,13 +465,16 @@ static int dm_blk_ioctl(struct block_device *bdev, 
> fmode_t mode,
>  
>   if (r > 0) {
>   /*
> -  * Target determined this ioctl is being issued against
> -  * a logical partition of the parent bdev; so extra
> -  * validation is needed.
> +  * Target determined this ioctl is being issued against a
> +  * subset of the parent bdev; require extra privileges.
>*/
> - r = scsi_verify_blk_ioctl(NULL, cmd);
> - if (r)
> + if (!capable(CAP_SYS_RAWIO)) {
> + DMWARN_LIMIT(
> + "%s: sending ioctl %x to DM device without required privilege.\n",
> + current->comm, cmd);
> + r = -ENOIOCTLCMD;
>   goto out;
> + }
>   }
>  
>   r =  __blkdev_driver_ioctl(bdev, mode, cmd, arg);
> 

Acked-by: Paolo Bonzini 

Thanks,

Paolo

Re: [PATCH v3] dm: don't allow ioctls to targets that don't map to whole devices

[Christoph Hellwig]

On Fri, Feb 03, 2017 at 11:39:22AM -0500, Mike Snitzer wrote:
> I assume you meant for v3 to remove the newline? ;)

I did.  And I swear I did edit the file, but I guess the ammend
didn't work.  I guess it's time for the weekend..  I'll resend after
I got some rest.

Re: [PATCH v3] dm: don't allow ioctls to targets that don't map to whole devices

[Mike Snitzer]

On Fri, Feb 03 2017 at 11:37am -0500,
Christoph Hellwig  wrote:

> .. at least for unprivileged users.  Before we called into the SCSI
> ioctl code to allow excemptions for a few SCSI passthrough ioctls,
> but this is pretty unsafe and except for this call dm knows nothing
> about SCSI ioctls.
> 
> As the SCSI ioctl code is now optional, we really don't want to
> drag it in for DM, and the exception is not very useful anyway.
> 
> Signed-off-by: Christoph Hellwig 
> Acked-by: Mike Snitzer 
> ---
>  drivers/md/dm.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 9e958bc94fed..fd4331aa2e19 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -465,13 +465,16 @@ static int dm_blk_ioctl(struct block_device *bdev, 
> fmode_t mode,
>  
>   if (r > 0) {
>   /*
> -  * Target determined this ioctl is being issued against
> -  * a logical partition of the parent bdev; so extra
> -  * validation is needed.
> +  * Target determined this ioctl is being issued against a
> +  * subset of the parent bdev; require extra privileges.
>*/
> - r = scsi_verify_blk_ioctl(NULL, cmd);
> - if (r)
> + if (!capable(CAP_SYS_RAWIO)) {
> + DMWARN_LIMIT(
> + "%s: sending ioctl %x to DM device without required privilege.\n",

I assume you meant for v3 to remove the newline? ;)