[PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

[PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

[Mike Snitzer]

On Tue, Mar 13 2018 at  5:23am -0400,
Denis Semakin <d.semakin@omprussia.ru> wrote:

> Hello.
> Here is fixed patch for modern 4.1x kernels.
> The idea is to forward secure erase request within device mapper layer to
> block device driver which can support secure erase.
> Could you please review?

There were various issues with your patch that I cleaned up, please see
the following.

But I'm left skeptical that this is enough.  Don't targets need to
explicitly handle these REQ_OP_SECURE_ERASE requests?  Similar to how
REQ_OP_DISCARD is handled?

I'd feel safer about having targets opt-in with setting (a new)
ti->num_secure_erase_bios.

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE
bios?

Mike


From: Denis Semakin <d.semakin@omprussia.ru>
Date: Tue, 13 Mar 2018 13:23:45 +0400
Subject: [PATCH] dm table: add support for secure erase forwarding

Set QUEUE_FLAG_SECERASE in DM device's queue_flags if a DM table's
data devices support secure erase.

Signed-off-by: Denis Semakin <d.semakin@omprussia.ru>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-table.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 7eb3e2a..d857369 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1846,6 +1846,31 @@ static bool dm_table_supports_discards(struct dm_table *t)
 	return true;
 }
 
+static int device_not_secure_erase_capable(struct dm_target *ti,
+					   struct dm_dev *dev, sector_t start,
+					   sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_table_supports_secure_erase(struct dm_table *t)
+{
+       struct dm_target *ti;
+       unsigned int i;
+
+       for (i = 0; i < dm_table_get_num_targets(t); i++) {
+               ti = dm_table_get_target(t, i);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
+		       return false;
+       }
+
+       return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {
@@ -1867,6 +1892,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	} else
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+	if (dm_table_supports_secure_erase(t))
+		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
 	if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
 		wc = true;
 		if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))
-- 
1.7.4.4

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

[Denis Semakin]

Hi.
Soon or later everybody start to think about security.
One of the most frequently requirement is 100% reliable data deletion from
any device in case of compromising or loss or theft.

For this, drive and memory cards manufacturers provide ERASE and TRIM features
which can notice (inform) controller of the device to erase sectors
(write down only zeros or one or random data). Features can be triggered
by calling ioctl() requests or a mount options (like ext4 does). But this works
only with whole device -- /dev/sdX, /dev/mmcblk0pX...
But what if for some security reasons we need to secure delete a single file.
A file-system layer provide this functionality... one may call __blkdev_issue_discard()
with BLKDEV_DISCARD_SECURE flag.
But...
All this works well if there is no virtual layer (like device-mapper)
between file-system and block-layer, because if device driver supports
this feature it can set up related flag in request_queue flags.

I have ext4 lvm partitions on my test instance and a drive which can
secure erase sectors.
Without lvm it works, with lvm it doesn't.
That's the purpose if this patch - to provide the opportunity to secure erase
given sectors (through device-mapper layer, forward request) that were assigned for regular file.



>But I'm left skeptical that this is enough.  Don't targets need to
>explicitly handle these REQ_OP_SECURE_ERASE requests?  Similar to how
>REQ_OP_DISCARD is handled?

I think yes, REQ_OP_DISCARD will not secure erase the data and it can be possible
to get it from device.

>I'd feel safer about having targets opt-in with setting (a new)
>ti->num_secure_erase_bios.

Well... May it make sense but I didn't see any reasons to add it in patch.

>Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE
>bios?
I think first of all a linear target of course should have this. For others I'm not sure, I need
to investigate.

Hopefully, I answered to all your question.


Denis

----- Исходное сообщение -----
От: "snitzer" <snitzer@redhat.com>
Кому: "Denis Semakin" <d.semakin@omprussia.ru>
Копия: "dm-devel" <dm-devel@redhat.com>
Отправленные: Четверг, 22 Март 2018 г 18:10:46
Тема: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

On Tue, Mar 13 2018 at  5:23am -0400,
Denis Semakin <d.semakin@omprussia.ru> wrote:

> Hello.
> Here is fixed patch for modern 4.1x kernels.
> The idea is to forward secure erase request within device mapper layer to
> block device driver which can support secure erase.
> Could you please review?

There were various issues with your patch that I cleaned up, please see
the following.

But I'm left skeptical that this is enough.  Don't targets need to
explicitly handle these REQ_OP_SECURE_ERASE requests?  Similar to how
REQ_OP_DISCARD is handled?

I'd feel safer about having targets opt-in with setting (a new)
ti->num_secure_erase_bios.

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE
bios?

Mike


From: Denis Semakin <d.semakin@omprussia.ru>
Date: Tue, 13 Mar 2018 13:23:45 +0400
Subject: [PATCH] dm table: add support for secure erase forwarding

Set QUEUE_FLAG_SECERASE in DM device's queue_flags if a DM table's
data devices support secure erase.

Signed-off-by: Denis Semakin <d.semakin@omprussia.ru>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-table.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 7eb3e2a..d857369 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1846,6 +1846,31 @@ static bool dm_table_supports_discards(struct dm_table *t)
 	return true;
 }
 
+static int device_not_secure_erase_capable(struct dm_target *ti,
+					   struct dm_dev *dev, sector_t start,
+					   sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_table_supports_secure_erase(struct dm_table *t)
+{
+       struct dm_target *ti;
+       unsigned int i;
+
+       for (i = 0; i < dm_table_get_num_targets(t); i++) {
+               ti = dm_table_get_target(t, i);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
+		       return false;
+       }
+
+       return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {
@@ -1867,6 +1892,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	} else
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+	if (dm_table_supports_secure_erase(t))
+		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
 	if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
 		wc = true;
 		if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))
-- 
1.7.4.4
-- 


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

[Denis Semakin]

Additional.

>Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE
>bios?
I've tested only with linear targets, so I suppose it's needed to
check what kind of target(s) we have.


----- Исходное сообщение -----
От: "snitzer" <snitzer@redhat.com>
Кому: "Denis Semakin" <d.semakin@omprussia.ru>
Копия: "dm-devel" <dm-devel@redhat.com>
Отправленные: Четверг, 22 Март 2018 г 18:10:46
Тема: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

On Tue, Mar 13 2018 at  5:23am -0400,
Denis Semakin <d.semakin@omprussia.ru> wrote:

> Hello.
> Here is fixed patch for modern 4.1x kernels.
> The idea is to forward secure erase request within device mapper layer to
> block device driver which can support secure erase.
> Could you please review?

There were various issues with your patch that I cleaned up, please see
the following.

But I'm left skeptical that this is enough.  Don't targets need to
explicitly handle these REQ_OP_SECURE_ERASE requests?  Similar to how
REQ_OP_DISCARD is handled?

I'd feel safer about having targets opt-in with setting (a new)
ti->num_secure_erase_bios.

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE
bios?

Mike


From: Denis Semakin <d.semakin@omprussia.ru>
Date: Tue, 13 Mar 2018 13:23:45 +0400
Subject: [PATCH] dm table: add support for secure erase forwarding

Set QUEUE_FLAG_SECERASE in DM device's queue_flags if a DM table's
data devices support secure erase.

Signed-off-by: Denis Semakin <d.semakin@omprussia.ru>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-table.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 7eb3e2a..d857369 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1846,6 +1846,31 @@ static bool dm_table_supports_discards(struct dm_table *t)
 	return true;
 }
 
+static int device_not_secure_erase_capable(struct dm_target *ti,
+					   struct dm_dev *dev, sector_t start,
+					   sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_table_supports_secure_erase(struct dm_table *t)
+{
+       struct dm_target *ti;
+       unsigned int i;
+
+       for (i = 0; i < dm_table_get_num_targets(t); i++) {
+               ti = dm_table_get_target(t, i);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
+		       return false;
+       }
+
+       return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {
@@ -1867,6 +1892,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	} else
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+	if (dm_table_supports_secure_erase(t))
+		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
 	if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
 		wc = true;
 		if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))
-- 
1.7.4.4
-- 
Best regards,

Denis Semakin
Software Developer
Open Mobile Platform

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

[Denis Semakin]

>I'd feel safer about having targets opt-in with setting (a new)
>ti->num_secure_erase_bios.
May be add a new field "bool secure_erase_supported:1" in dm_target structure instead?
And set up it "true" in constructor for linear targets.


----- Исходное сообщение -----
От: "snitzer" <snitzer@redhat.com>
Кому: "Denis Semakin" <d.semakin@omprussia.ru>
Копия: "dm-devel" <dm-devel@redhat.com>
Отправленные: Четверг, 22 Март 2018 г 18:10:46
Тема: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

On Tue, Mar 13 2018 at  5:23am -0400,
Denis Semakin <d.semakin@omprussia.ru> wrote:

> Hello.
> Here is fixed patch for modern 4.1x kernels.
> The idea is to forward secure erase request within device mapper layer to
> block device driver which can support secure erase.
> Could you please review?

There were various issues with your patch that I cleaned up, please see
the following.

But I'm left skeptical that this is enough.  Don't targets need to
explicitly handle these REQ_OP_SECURE_ERASE requests?  Similar to how
REQ_OP_DISCARD is handled?

I'd feel safer about having targets opt-in with setting (a new)
ti->num_secure_erase_bios.

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE
bios?

Mike


From: Denis Semakin <d.semakin@omprussia.ru>
Date: Tue, 13 Mar 2018 13:23:45 +0400
Subject: [PATCH] dm table: add support for secure erase forwarding

Set QUEUE_FLAG_SECERASE in DM device's queue_flags if a DM table's
data devices support secure erase.

Signed-off-by: Denis Semakin <d.semakin@omprussia.ru>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-table.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 7eb3e2a..d857369 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1846,6 +1846,31 @@ static bool dm_table_supports_discards(struct dm_table *t)
 	return true;
 }
 
+static int device_not_secure_erase_capable(struct dm_target *ti,
+					   struct dm_dev *dev, sector_t start,
+					   sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_table_supports_secure_erase(struct dm_table *t)
+{
+       struct dm_target *ti;
+       unsigned int i;
+
+       for (i = 0; i < dm_table_get_num_targets(t); i++) {
+               ti = dm_table_get_target(t, i);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
+		       return false;
+       }
+
+       return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {
@@ -1867,6 +1892,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	} else
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+	if (dm_table_supports_secure_erase(t))
+		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
 	if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
 		wc = true;
 		if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))
-- 
1.7.4.4
-- 
Best regards,

Denis Semakin
Software Developer
Open Mobile Platform

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

[Denis Semakin]

Hi.
Soon or later everybody start to think about security.
One of the most frequently requirement is 100% reliable data deletion from
any device in case of compromising or loss or theft.

For this, drive and memory cards manufacturers provide ERASE and TRIM features
which can notice (inform) controller of the device to erase sectors
(write down only zeros or one or random data). Features can be triggered
by calling ioctl() requests or a mount options (like ext4 does). But this works
only with whole device -- /dev/sdX, /dev/mmcblk0pX...
But what if for some security reasons we need to secure delete a single file.
A file-system layer provide this functionality... one may call __blkdev_issue_discard()
with BLKDEV_DISCARD_SECURE flag.
But...
All this works well if there is no virtual layer (like device-mapper)
between file-system and block-layer, because if device driver supports
this feature it can set up related flag in request_queue flags.

I have ext4 lvm partitions on my test instance and a drive which can
secure erase sectors.
Without lvm it works, with lvm it doesn't.
That's the purpose if this patch - to provide the opportunity to secure erase
given sectors (through device-mapper layer, forward request) that were assigned for regular file.



>But I'm left skeptical that this is enough.  Don't targets need to
>explicitly handle these REQ_OP_SECURE_ERASE requests?  Similar to how
>REQ_OP_DISCARD is handled?

I think yes, REQ_OP_DISCARD will not secure erase the data and it can be possible
to get it from device.

>I'd feel safer about having targets opt-in with setting (a new)
>ti->num_secure_erase_bios.

Well... May it make sense but I didn't see any reasons to add it in patch.

>Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE
>bios?
I think first of all a linear target of course should have this. For others I'm not sure, I need
to investigate.

Hopefully, I answered to all your question.


Denis

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

[Denis Semakin]

Additional.

>Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE
>bios?
I've tested only with linear targets, so I suppose it's needed to
check what kind of target(s) we have.

Denis

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

[Denis Semakin]

>I'd feel safer about having targets opt-in with setting (a new)
>ti->num_secure_erase_bios.
May be add a new field "bool secure_erase_supported:1" in dm_target structure instead?
And set up it "true" in constructor for linear targets.

Denis