Patchwork Allow ATA_ passthrough command through sg.

login
register
mail settings
Submitter Gwendal Grignou
Date June 29, 2009, 9:24 p.m.
Message ID <1246310665-31727-1-git-send-email-gwendal@google.com>
Download mbox | patch
Permalink /patch/29298/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Gwendal Grignou - June 29, 2009, 9:24 p.m.
We can already send ATA specific commands using /dev/sd device files.
This patch allow to use /dev/sg device files as well.

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 block/scsi_ioctl.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Alan Cox - June 29, 2009, 10:20 p.m.
On Mon, 29 Jun 2009 14:24:25 -0700
Gwendal Grignou <gwendal@google.com> wrote:

> We can already send ATA specific commands using /dev/sd device files.
> This patch allow to use /dev/sg device files as well.

That seems a very very bad idea. The point of the filters is only to
allow through commands that are safe for all users with read or write
permission to use.

Backdooring it with arbitary passthrough ATA12/ATA16 commands doesn't
seem very wise.

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Garzik - June 29, 2009, 11:26 p.m.
Alan Cox wrote:
> On Mon, 29 Jun 2009 14:24:25 -0700
> Gwendal Grignou <gwendal@google.com> wrote:
> 
>> We can already send ATA specific commands using /dev/sd device files.
>> This patch allow to use /dev/sg device files as well.
> 
> That seems a very very bad idea. The point of the filters is only to
> allow through commands that are safe for all users with read or write
> permission to use.
> 
> Backdooring it with arbitary passthrough ATA12/ATA16 commands doesn't
> seem very wise.

Indeed.  This pretty much defeats the filter for all ATA commands, 
including ones that can brick your drive.

	Jeff



--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gwendal Grignou - June 29, 2009, 11:39 p.m.
Sorry for the noise: I thought I needed that change to send ATA
passthrough commands as root. The change is indeed useless and
dangerous.

Gwendal.

On Mon, Jun 29, 2009 at 4:26 PM, Jeff Garzik<jeff@garzik.org> wrote:
> Alan Cox wrote:
>>
>> On Mon, 29 Jun 2009 14:24:25 -0700
>> Gwendal Grignou <gwendal@google.com> wrote:
>>
>>> We can already send ATA specific commands using /dev/sd device files.
>>> This patch allow to use /dev/sg device files as well.
>>
>> That seems a very very bad idea. The point of the filters is only to
>> allow through commands that are safe for all users with read or write
>> permission to use.
>>
>> Backdooring it with arbitary passthrough ATA12/ATA16 commands doesn't
>> seem very wise.
>
> Indeed.  This pretty much defeats the filter for all ATA commands, including
> ones that can brick your drive.
>
>        Jeff
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Douglas Gilbert - June 30, 2009, 12:21 a.m.
Strange, my utilities and packages like smartmontools
have been sending them through for at least three years.

I presume this patch allows non-root users to send these
commands. Might there be security implications if ATA
WRITE commands are sent through? Non-root users would
still need permissions on the sg device node (e.g.
/dev/sg1).

Doug Gilbert


Gwendal Grignou wrote:
> We can already send ATA specific commands using /dev/sd device files.
> This patch allow to use /dev/sg device files as well.
> 
> Signed-off-by: Gwendal Grignou <gwendal@google.com>
> ---
>  block/scsi_ioctl.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> index 82a0ca2..93fa53e 100644
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -186,6 +186,12 @@ void blk_set_cmd_filter_defaults(struct blk_cmd_filter *filter)
>  	__set_bit(GPCMD_LOAD_UNLOAD, filter->write_ok);
>  	__set_bit(GPCMD_SET_STREAMING, filter->write_ok);
>  	__set_bit(GPCMD_SET_READ_AHEAD, filter->write_ok);
> +
> +	/* ATA Passthrough */
> +	__set_bit(ATA_12, filter->read_ok);
> +	__set_bit(ATA_12, filter->write_ok);
> +	__set_bit(ATA_16, filter->read_ok);
> +	__set_bit(ATA_16, filter->write_ok);
>  }
>  EXPORT_SYMBOL_GPL(blk_set_cmd_filter_defaults);
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 82a0ca2..93fa53e 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -186,6 +186,12 @@  void blk_set_cmd_filter_defaults(struct blk_cmd_filter *filter)
 	__set_bit(GPCMD_LOAD_UNLOAD, filter->write_ok);
 	__set_bit(GPCMD_SET_STREAMING, filter->write_ok);
 	__set_bit(GPCMD_SET_READ_AHEAD, filter->write_ok);
+
+	/* ATA Passthrough */
+	__set_bit(ATA_12, filter->read_ok);
+	__set_bit(ATA_12, filter->write_ok);
+	__set_bit(ATA_16, filter->read_ok);
+	__set_bit(ATA_16, filter->write_ok);
 }
 EXPORT_SYMBOL_GPL(blk_set_cmd_filter_defaults);