[RFC] libata-scsi: introducing SANITIZE translation

[RFC] libata-scsi: introducing SANITIZE translation

[tom.ty89]

From: Tom Yan <tom.ty89@gmail.com>

With this patch, users can make use of the SANITIZE DEVICE feature
set through utility like sg_sanitize.

Support for BLOCK ERASE, CRYPTOGRAPHIC ERASE and EXIT FAILURE MODE
has been implemented. Support for OVERWRITE that involves a
parameter list has been left out for now.

Further support for command with IMMED bit set to zero, REQUEST
SENSE translation for user-space status polling, and support
checking in IDENTIFY DEVICE data log (return proper sense data
when designated method is not supported) should be implemented
in the future as well.

`sg_sanitize -e -B|-C|-F /dev/sdX` should work fine with this.

Signed-off-by: Tom Yan <tom.ty89@gmail.com>

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bfec66f..a64991b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3346,6 +3346,63 @@ invalid_opcode:
 	return 1;
 }
 
+static unsigned int ata_scsi_sanitize_xlat(struct ata_queued_cmd *qc) {
+	struct ata_taskfile *tf = &qc->tf;
+	struct scsi_cmnd *scmd = qc->scsicmd;
+	struct ata_device *dev = qc->dev;
+	const u8 *cdb = scmd->cmnd;
+	u16 fp;
+	u8 bp = 0xff;
+
+	/* for now we only support SANITIZE with IMMED bit set */
+	if (unlikely(!(cdb[1] & 0x80))) {
+		fp = 1;
+		bp = 7;
+		goto invalid_fld;
+	}
+
+	tf->protocol = ATA_PROT_NODATA;
+	tf->command = ATA_CMD_SANITIZE_DEVICE;
+	tf->hob_nsect |= (cdb[1] & 0x40) << 1;
+	tf->nsect |= (cdb[1] & 0x20) >> 1;
+	tf->flags |= ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR;
+
+	switch (cdb[1] & 0x1f) {
+	/* TODO: add support for OVERWRITE */
+	case 0x2: /* BLOCK ERASE */
+		tf->hob_feature = 0x0;
+		tf->feature = 0x12;
+		tf->hob_lbal = 0x42;
+		tf->lbah = 0x6b;
+		tf->lbam = 0x45;
+		tf->lbal = 0x72;
+		break;
+	case 0x3: /* CRYPTOGRAPHIC ERASE */
+		tf->hob_feature = 0x0;
+		tf->feature = 0x11;
+		tf->hob_lbal = 0x43;
+		tf->lbah = 0x72;
+		tf->lbam = 0x79;
+		tf->lbal = 0x70;
+		break;
+	case 0x1f: /* EXIT FAILURE MODE */
+		tf->hob_feature = 0x0;
+		tf->feature = 0x0;
+		tf->nsect |= 0x1;
+		break;
+	default:
+		fp = 1;
+		bp = 4;
+		goto invalid_fld;
+	}
+
+	return 0;
+
+invalid_fld:
+	ata_scsi_set_invalid_field(dev, scmd, fp, bp);
+	return 1;
+}
+
 /**
  *	ata_scsi_report_zones_complete - convert ATA output
  *	@qc: command structure returning the data
@@ -3869,6 +3926,9 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd)
 	case WRITE_SAME_16:
 		return ata_scsi_write_same_xlat;
 
+	case SANITIZE:
+		return ata_scsi_sanitize_xlat;
+
 	case SYNCHRONIZE_CACHE:
 		if (ata_try_flush_cache(dev))
 			return ata_scsi_flush_xlat;
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index d1defd1..20d6085 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -72,6 +72,7 @@
 #define UNMAP		      0x42
 #define READ_TOC              0x43
 #define READ_HEADER           0x44
+#define SANITIZE              0x48
 #define GET_EVENT_STATUS_NOTIFICATION 0x4a
 #define LOG_SELECT            0x4c
 #define LOG_SENSE             0x4d
-- 
2.9.0

Re: [RFC] libata-scsi: introducing SANITIZE translation

[James Bottomley]

On Fri, 2016-07-08 at 00:32 +0800, tom.ty89@gmail.com wrote:
> From: Tom Yan <tom.ty89@gmail.com>
> 
> With this patch, users can make use of the SANITIZE DEVICE feature
> set through utility like sg_sanitize.
> 
> Support for BLOCK ERASE, CRYPTOGRAPHIC ERASE and EXIT FAILURE MODE
> has been implemented. Support for OVERWRITE that involves a
> parameter list has been left out for now.
> 
> Further support for command with IMMED bit set to zero, REQUEST
> SENSE translation for user-space status polling, and support
> checking in IDENTIFY DEVICE data log (return proper sense data
> when designated method is not supported) should be implemented
> in the future as well.
> 
> `sg_sanitize -e -B|-C|-F /dev/sdX` should work fine with this.

Why on earth would you want to do this?  If your intent is to sanitise
the disk using a cryptographic erase you presumably have a real
security need for doing it and, knowing what goes into most SAT layers,
I'd not really trust any SAT for this operation, so for an underlying
SATA device I'd use ATA_16 to send a real ACS-2 SANITIZE command.

Just as a general note about our SAT layer: Adding little used features
is an invitation to bloat it with buggy implementations which makes it
harder to understand and bug prone for odd and unlikely use cases,
which then take ages to diagnose and track down.  The only things which
should be in the SAT is what the Linux SCSI subsystem would actually
use.  For everything else, if the user cares enough, they'll send down
an encapsulated ATA command.

James

Re: [RFC] libata-scsi: introducing SANITIZE translation

[Tom Yan]

To be honest, that sounds like FUD to me.

The exact reason why this can "safely" be introduced to the SATL is
that, it is a "one-shot" command that would only be triggered by the
user through a user space utility. It's nothing like TRIM that would
be triggered by the filesystem layer or so "from time to time", which
might cost users "unexpected" data loss. It is also nothing like
SECURE ERASE (which vendors had to abuse to provide equivalence of
BLOCK ERASE and CRYPTOGRAPHIC ERASE for SSDs and drives that does
hardware encryption) that requires the drive to be locked before you
can issue an erase command.

Certainly you can expect users to pack an ATA PASS-THROUGH command
themselves and issued it with sg_raw or so, or hdparm to be patched to
support the full feature set (including the "optional"
FREEZE/ANTIFREEZE sub-feature), but I don't see how these would be
reasons that the translation should not be introduced to the kernel,
so that we can make good use of its existing SATL infrastructure and
sg_sanitize.

Whether it is "secure" enough to use the implementation would be the
user's call. To be frank, saying that implementing it in the "SAT-way"
would make it untrustable doesn't make any sense to me, especially
when the way the feature set works is considered.

On 8 July 2016 at 00:47, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Fri, 2016-07-08 at 00:32 +0800, tom.ty89@gmail.com wrote:
>> From: Tom Yan <tom.ty89@gmail.com>
>>
>> With this patch, users can make use of the SANITIZE DEVICE feature
>> set through utility like sg_sanitize.
>>
>> Support for BLOCK ERASE, CRYPTOGRAPHIC ERASE and EXIT FAILURE MODE
>> has been implemented. Support for OVERWRITE that involves a
>> parameter list has been left out for now.
>>
>> Further support for command with IMMED bit set to zero, REQUEST
>> SENSE translation for user-space status polling, and support
>> checking in IDENTIFY DEVICE data log (return proper sense data
>> when designated method is not supported) should be implemented
>> in the future as well.
>>
>> `sg_sanitize -e -B|-C|-F /dev/sdX` should work fine with this.
>
> Why on earth would you want to do this?  If your intent is to sanitise
> the disk using a cryptographic erase you presumably have a real
> security need for doing it and, knowing what goes into most SAT layers,
> I'd not really trust any SAT for this operation, so for an underlying
> SATA device I'd use ATA_16 to send a real ACS-2 SANITIZE command.
>
> Just as a general note about our SAT layer: Adding little used features
> is an invitation to bloat it with buggy implementations which makes it
> harder to understand and bug prone for odd and unlikely use cases,
> which then take ages to diagnose and track down.  The only things which
> should be in the SAT is what the Linux SCSI subsystem would actually
> use.  For everything else, if the user cares enough, they'll send down
> an encapsulated ATA command.
>
> James
>
>

Re: [RFC] libata-scsi: introducing SANITIZE translation

[James Bottomley]

On Sat, 2016-07-09 at 00:20 +0800, Tom Yan wrote:
> To be honest, that sounds like FUD to me.

If argument is to be in acronyms, it's KISS.

> The exact reason why this can "safely" be introduced to the SATL is
> that, it is a "one-shot" command that would only be triggered by the
> user through a user space utility. It's nothing like TRIM that would
> be triggered by the filesystem layer or so "from time to time", which
> might cost users "unexpected" data loss. It is also nothing like
> SECURE ERASE (which vendors had to abuse to provide equivalence of
> BLOCK ERASE and CRYPTOGRAPHIC ERASE for SSDs and drives that does
> hardware encryption) that requires the drive to be locked before you
> can issue an erase command.

OK, since you ignore the argument about maintenance: safety for us
means that it doesn't bitrot as an almost never used addition.  The
reason our SATL should only support the commands Linux uses is
precisely because if it's used often we get immediate notice of when we
break it.  This maintenance burden means that adding stuff isn't free
so we should have some utility bar before we do it.  Just "because we
can" doesn't seem to rise to that.

> Certainly you can expect users to pack an ATA PASS-THROUGH command
> themselves and issued it with sg_raw or so, or hdparm to be patched 
> to support the full feature set (including the "optional"
> FREEZE/ANTIFREEZE sub-feature), but I don't see how these would be
> reasons that the translation should not be introduced to the kernel,
> so that we can make good use of its existing SATL infrastructure and
> sg_sanitize.

Or we could simply patch sg_sanitze to issue the ATA_16 pass through
when it sees a sata device ...

> Whether it is "secure" enough to use the implementation would be the
> user's call. To be frank, saying that implementing it in the "SAT
> -way"
> would make it untrustable doesn't make any sense to me, especially
> when the way the feature set works is considered.

To be honest, I bet real security people won't even trust the drive
firmware.  Their answer will still be dd random patterns to prevent
easy retrieval then crush the drive to prevent forensic retrieval.

James


> On 8 July 2016 at 00:47, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Fri, 2016-07-08 at 00:32 +0800, tom.ty89@gmail.com wrote:
> > > From: Tom Yan <tom.ty89@gmail.com>
> > > 
> > > With this patch, users can make use of the SANITIZE DEVICE
> > > feature
> > > set through utility like sg_sanitize.
> > > 
> > > Support for BLOCK ERASE, CRYPTOGRAPHIC ERASE and EXIT FAILURE
> > > MODE
> > > has been implemented. Support for OVERWRITE that involves a
> > > parameter list has been left out for now.
> > > 
> > > Further support for command with IMMED bit set to zero, REQUEST
> > > SENSE translation for user-space status polling, and support
> > > checking in IDENTIFY DEVICE data log (return proper sense data
> > > when designated method is not supported) should be implemented
> > > in the future as well.
> > > 
> > > `sg_sanitize -e -B|-C|-F /dev/sdX` should work fine with this.
> > 
> > Why on earth would you want to do this?  If your intent is to
> > sanitise
> > the disk using a cryptographic erase you presumably have a real
> > security need for doing it and, knowing what goes into most SAT
> > layers,
> > I'd not really trust any SAT for this operation, so for an
> > underlying
> > SATA device I'd use ATA_16 to send a real ACS-2 SANITIZE command.
> > 
> > Just as a general note about our SAT layer: Adding little used
> > features
> > is an invitation to bloat it with buggy implementations which makes
> > it
> > harder to understand and bug prone for odd and unlikely use cases,
> > which then take ages to diagnose and track down.  The only things
> > which
> > should be in the SAT is what the Linux SCSI subsystem would
> > actually
> > use.  For everything else, if the user cares enough, they'll send
> > down
> > an encapsulated ATA command.
> > 
> > James
> > 
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC] libata-scsi: introducing SANITIZE translation

[Tom Yan]

On 8 July 2016 at 17:29, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> OK, since you ignore the argument about maintenance: safety for us
> means that it doesn't bitrot as an almost never used addition.  The
> reason our SATL should only support the commands Linux uses is
> precisely because if it's used often we get immediate notice of when we
> break it.  This maintenance burden means that adding stuff isn't free
> so we should have some utility bar before we do it.  Just "because we
> can" doesn't seem to rise to that.
>

Well yeah it might rot just like other parts of the current SATL, but
at least its rot would be self-contained. I don't really find it "an
almost never used addition" btw. It's just a handy feature set that no
one has implemented any interface for users to make use of it.

>
> Or we could simply patch sg_sanitze to issue the ATA_16 pass through
> when it sees a sata device ...
>

Ugh that sounds ugly to me. Anyway that's off-topic.

>
> To be honest, I bet real security people won't even trust the drive
> firmware.  Their answer will still be dd random patterns to prevent
> easy retrieval then crush the drive to prevent forensic retrieval.
>

You know what, I don't think the feature set is totally about
"security" anyway. It's just a handy way quickly (that's partly why I
didn't bother to implement translation for OVERWRITE) erase all the
data. Also, whether one considers it secure enough or not would be
purely his opinion under the usage context of his own, as always.

Re: [RFC] libata-scsi: introducing SANITIZE translation

[James Bottomley]

On Fri, 2016-07-08 at 19:38 +0000, Tom Yan wrote:
> On 8 July 2016 at 17:29, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > Or we could simply patch sg_sanitze to issue the ATA_16 pass 
> > through when it sees a sata device ...
> > 
> 
> Ugh that sounds ugly to me. Anyway that's off-topic.

Not really.  The point is that you've proposed something as an addition
to the kernel that can also be done in userspace.  Checking if it can
work easily there is like a barrier to entry.  If it works, then fine,
we're done.  If it throws up problems then we reconsider the kernel
route.

James

Re: [RFC] libata-scsi: introducing SANITIZE translation

[Tom Yan]

I don't suppose there would be any problem doing it in userspace /
with ATA PASS-THROUGH anyway. I just couldn't agree that it would be
the reason not to implement the translation (which covers the core
part of the feature set) in the kernel. But certainly I wouldn't keep
aruging on this.

I don't think we would really want to patch sg_sanitize the way you
proposed, otherwise we would have made the SCSI disk driver doing ATA
IDENTIFY DEVICE and issue TRIM commands directly, instead of relying
on a SATL. ATA PASS-THROUGH should never be "triggered" like that
anyway.

Well, it is alright to have an "sg_sat_sanitize" (which makes use of
ATA PASS-THROUGH like other sg_sat_*) though. I am just not sure if
sg3_utils should go on having more sg_sat_*...

On 9 July 2016 at 08:49, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Fri, 2016-07-08 at 19:38 +0000, Tom Yan wrote:
>> On 8 July 2016 at 17:29, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>> > Or we could simply patch sg_sanitze to issue the ATA_16 pass
>> > through when it sees a sata device ...
>> >
>>
>> Ugh that sounds ugly to me. Anyway that's off-topic.
>
> Not really.  The point is that you've proposed something as an addition
> to the kernel that can also be done in userspace.  Checking if it can
> work easily there is like a barrier to entry.  If it works, then fine,
> we're done.  If it throws up problems then we reconsider the kernel
> route.
>
> James
>

Re: [RFC] libata-scsi: introducing SANITIZE translation

[Mark Lord]

On 16-07-11 02:35 AM, Tom Yan wrote:
> I don't suppose there would be any problem doing it in userspace /
> with ATA PASS-THROUGH anyway.
..
>>> On 8 July 2016 at 17:29, James Bottomley
>>> <James.Bottomley@hansenpartnership.com> wrote:
..
>> Not really.  The point is that you've proposed something as an addition
>> to the kernel that can also be done in userspace.  Checking if it can
>> work easily there is like a barrier to entry.  If it works, then fine,
>> we're done.  If it throws up problems then we reconsider the kernel
>> route.

hdparm has full support for the SANITIZE commands in userspace.

-ml