dm: add secdel target
Subsystems (3)
PATCH 1/1 dm: add secdel target
Report to the upper level ability to discard, and translate arriving discards to the writes of random or zero data to the underlying level.
---
This target is the same as the linear target except that is reports ability to discard to the upper level and translates arriving discards into sector overwrites with random (or zero) data.
The target does not try to determine if the underlying drive reliably supports data overwrites, this decision is solely on the discretion of a user.
It may be useful to create a secure deletion setup when filesystem when unlinking a file sends discards to its sectors, in this target they are translated to writes that wipe deleted data on the underlying drive.
Tested on x86.
Documentation/device-mapper/dm-secdel.txt | 24 ++ drivers/md/Kconfig | 14 ++ drivers/md/Makefile | 2 + drivers/md/dm-secdel.c | 399 ++++++++++++++++++++++++++++++ 4 files changed, 439 insertions(+) create mode 100644 Documentation/device-mapper/dm-secdel.txt create mode 100644 drivers/md/dm-secdel.c
Documentation/device-mapper/dm-secdel.txt new file +24 -0
@@ -0,0 +1,24 @@ +dm-secdel +========= + +This target is the same as the linear target except that is reports ability to +discard to the upper level and translates arriving discards into sector +overwrites with random (or zero) data. + +The target does not try to determine if the underlying drive reliably supports +data overwrites, this decision is solely on the discretion of a user. Please +note that not all drivers support this ability. + +It may be useful to create a secure deletion setup when filesystem (which +supports discards, such as ext4, and mounted in discard mode) when unlinking a +file sends discards to its sectors, in this target they are translated to +writes that wipe deleted data on the underlying drive. + + +Parameters: <dev path> <offset> [<mode>] + <dev path>: Full pathname to the underlying block-device, or a + "major:minor" device-number. + <offset>: Starting sector within the device. + <mode>: Optional overwriting mode specifier: + "rand" to overwrite with random data, + "zero" to overwrite with zeros (default).
drivers/md/Kconfig +14 -0
@@ -559,4 +559,18 @@ config DM_ZONED If unsure, say N. +config DM_SECDEL + tristate "Secdel target" + depends on BLK_DEV_DM + help + + A target that transforms discards into overwrites with random data or + zeros. It may be useful if your underlying device supports data + overwriting to wipe deleted data (if the filesystem supports + discards). This device does not attempt to detect if the underlying + drive supports data overwriting reliably, this decision is solely on + the discretion of an end user. + + If unsure, say N. + endif # MD
drivers/md/Makefile +2 -0
@@ -25,6 +25,7 @@ dm-zoned-y += dm-zoned-target.o dm-zoned-metadata.o dm-zoned-reclaim.o linear-y += md-linear.o multipath-y += md-multipath.o faulty-y += md-faulty.o +secdel-y += md-secdel.o # Note: link order is important. All raid personalities # and must come before md.o, as they each initialise
@@ -68,6 +69,7 @@ obj-$(CONFIG_DM_LOG_WRITES) += dm-log-writes.o obj-$(CONFIG_DM_INTEGRITY) += dm-integrity.o obj-$(CONFIG_DM_ZONED) += dm-zoned.o obj-$(CONFIG_DM_WRITECACHE) += dm-writecache.o +obj-$(CONFIG_DM_SECDEL) += dm-secdel.o ifeq ($(CONFIG_DM_UEVENT),y) dm-mod-objs += dm-uevent.o
drivers/md/dm-secdel.c new file +399 -0
399 lines not on this page · raw message
Signature · 1 line
2.11.0
Quoted 8 lines · Vitaly Chikunov
Report to the upper level ability to discard, and translate arriving discards to the writes of random or zero data to the underlying level.
Signed-off-by Vitaly Chikunov <[email protected]>---
This target is the same as the linear target except that is reports ability to discard to the upper level and translates arriving discards into sector overwrites with random (or zero) data.
There is a fair amount of code duplication between dm-linear.c and this new target.
Something needs to give, ideally you'd factor out methods that are shared by both targets, but those methods must _not_ introduce overhead to dm-linear.
Could be that dm-linear methods just get called by the wrapper dm-sec-erase target (more on the "dm-sec-erase" name below).
Quoted 8 lines
The target does not try to determine if the underlying drive reliably supports data overwrites, this decision is solely on the discretion of a user.
It may be useful to create a secure deletion setup when filesystem when unlinking a file sends discards to its sectors, in this target they are translated to writes that wipe deleted data on the underlying drive.
Tested on x86.
All of this extra context and explanation needs to be captured in the actual patch header. Not as a tangent in that "cut" section of your patch header.
Quoted 7 lines
Documentation/device-mapper/dm-secdel.txt | 24 ++ drivers/md/Kconfig | 14 ++ drivers/md/Makefile | 2 + drivers/md/dm-secdel.c | 399 ++++++++++++++++++++++++++++++ 4 files changed, 439 insertions(+) create mode 100644 Documentation/device-mapper/dm-secdel.txt create mode 100644 drivers/md/dm-secdel.c
<snip>
Shouldn't this target be implementing all that is needed for REQ_OP_SECURE_ERASE support? And the resulting DM device would advertise its capability using QUEUE_FLAG_SECERASE?
And this is why I think the target should be named "dm-sec-erase" or even "dm-secure-erase".
Quoted 5 lines
drivers/md/dm-secdel.c new file +0
-0
<snip>
Quoted 5 lines
drivers/md/dm-secdel.c new file +5
-0+/* +* Send amount of masking data to the device +* @mode: 0 to write zeros, otherwise to write random data +*/ +static int issue_erase(struct block_device *bdev, sector_t sector,
drivers/md/dm-secdel.c line 163
Quoted 10 lines
drivers/md/dm-secdel.c new file +10
-0+ sector_t nr_sects, gfp_t gfp_mask, enum secdel_mode mode) +{ + int ret = 0; + +while (nr_sects) { +struct bio *bio; +unsigned int nrvecs =min (nr_sects, + (sector_t )BIO_MAX_PAGES >>3 ); + + bio =bio_alloc (gfp_mask, nrvecs);
You should probably be using your own bioset to allocate these bios.
Quoted 12 lines
drivers/md/dm-secdel.c new file +12
-0+if (!bio) { +DMERR ("%s %lu[%lu]: no memory to allocate bio (%u)" , + __func__, sector, nr_sects, nrvecs); + ret = -ENOMEM ; +break ; + } + + bio->bi_iter.bi_sector = sector; +bio_set_dev (bio, bdev); + bio->bi_end_io = bio_end_erase; + +while (nr_sects !=0 ) {
drivers/md/dm-secdel.c line 185
Quoted 10 lines
drivers/md/dm-secdel.c new file +10
-0+unsigned int sn; +struct page *page =NULL ; + + sn =min ((sector_t )PAGE_SIZE >>9 , nr_sects); +if (mode ==SECDEL_MODE_RAND ) { + page =alloc_page (gfp_mask); +if (!page) { +DMERR ("%s %lu[%lu]: no memory to allocate page for random data" , + __func__, sector, nr_sects); +/* will fallback to zero filling */
In general, performing memory allocations to service IO is something all DM core and DM targets must work to avoid. This smells bad.
...
Quoted 12 lines
drivers/md/dm-secdel.c new file +12
-0+ +/* convert discards into writes */ +static int secdel_map_discard (struct dm_target *ti,struct bio *sbio) +{ +struct secdel_c *lc = ti->private; +struct block_device *bdev = lc->dev->bdev; +sector_t sector = sbio->bi_iter.bi_sector; +sector_t nr_sects =bio_sectors (sbio); + + lc->requests++; +if (!bio_sectors (sbio)) +return 0 ;
drivers/md/dm-secdel.c line 232
Quoted 10 lines
drivers/md/dm-secdel.c new file +10
-0+if (!op_discard (sbio)) +return 0 ; + lc->discards++; +if (WARN_ON (sbio->bi_vcnt !=0 )) +return -1 ; +DMDEBUG ("DISCARD %lu: %u sectors M%d" , sbio->bi_iter.bi_sector, +bio_sectors (sbio), lc->mode); +bio_endio (sbio); + +if (issue_erase (bdev, sector, nr_sects,GFP_NOFS , lc->mode))
At a minimum this should be GFP_NOIO. You don't want to recurse into block (and potentially yourself) in the face of low memory.
drivers/md/dm-secdel.c line 254
Quoted 10 lines
drivers/md/dm-secdel.c new file +10
-0+static int secdel_end_io (struct dm_target *ti,struct bio *bio, +blk_status_t *error) +{ +struct secdel_c *lc = ti->private; + +if (!*error &&bio_op (bio) ==REQ_OP_ZONE_REPORT ) +dm_remap_zone_report (ti, bio, lc->start); + +return DM_ENDIO_DONE ; +}
Perfect example of why this new target shouldn't be doing a point in time copy of dm-linear code.
With 4.20's upcoming zoned device changes dm-linear no longer even has an end_io hook (which was introduced purely for REQ_OP_ZONE_REPORT's benefit).
Mike
Quoted 12 lines · Mike Snitzer
Quoted 8 lines · Vitaly Chikunov
Report to the upper level ability to discard, and translate arriving discards to the writes of random or zero data to the underlying level.
Signed-off-by Vitaly Chikunov <[email protected]>---
This target is the same as the linear target except that is reports ability to discard to the upper level and translates arriving discards into sector overwrites with random (or zero) data.
Quoted 6 lines
There is a fair amount of code duplication between
dm-linear.cand this new target.Something needs to give, ideally you'd factor out methods that are shared by both targets, but those methods must _not_ introduce overhead to dm-linear.
I see three possible solutions:
- integrate 'secdel' into dm-linear;
- export functions like linear_dax_copy_to_iter and fill them into secdel's struct target_type;
- call methods of linear's struct target_type from secdel's one.
Quoted 19 lines
Could be that dm-linear methods just get called by the wrapper dm-sec-erase target (more on the "dm-sec-erase" name below).
Quoted 5 lines
drivers/md/dm-secdel.c new file +0
-0...
drivers/md/dm-secdel.c line 185
Quoted 6 lines
drivers/md/dm-secdel.c new file +6
-0+if (mode ==SECDEL_MODE_RAND ) { + page =alloc_page (gfp_mask); +if (!page) { +DMERR ("%s %lu[%lu]: no memory to allocate page for random data" , + __func__, sector, nr_sects); +/* will fallback to zero filling */ In general, performing memory allocations to service IO is something all DM core and DM targets must work to avoid. This smells bad.
Quoted 1 line
...
Yes, but we need to create and write random data, so I don't see other solution. In zero overwriting mode page is not allocated. And it's fall back to zero overwriting if page allocation fails for random data.
Quoted 1 line
Mike
I will address these and other your concerns in v2.
Thanks!
Just as a note: the name is a complete misowner, a couple overwrite are not in any way secure deletion. So naming it this way and exposing this as erase is a problem that is going to get back to bite us.
If you really want this anyway at least give it a different way, and do a one-time warning when th first erase comes in that it is not in any meaninful way secure.
Quoted 3 lines · Christoph Hellwig
Just as a note: the name is a complete misowner, a couple overwrite are not in any way secure deletion. So naming it this way and exposing this as erase is a problem that is going to get back to bite us.
In what way it's not secure deletion?
It's secure deletion by overwriting discarded data instead of leaving it as is. Thus it's secure deletion in some way. Level of security and applicability (disks choice) is to be determined by the end user. Because nobody could guarantee absolute security. Some three letter agencies require just one pass of overwrite, some say that more than one pass does not increase security. Some hardware disks advertising secure deletion may do not much more than this target. Thus 'secure erase' is applicable in that way too.
Quoted 3 lines
If you really want this anyway at least give it a different way, and do a one-time warning when th first erase comes in that it is not in any meaninful way secure.
dm-erase or dm-wipe? dm-discerase? But still provide REQ_OP_SECURE_ERASE support?
Quoted 9 lines · Vitaly Chikunov
Quoted 3 lines · Christoph Hellwig
Just as a note: the name is a complete misowner, a couple overwrite are not in any way secure deletion. So naming it this way and exposing this as erase is a problem that is going to get back to bite us.
In what way it's not secure deletion?
It's secure deletion by overwriting discarded data instead of leaving it as is.
Overwriting data does not delete data. Most certainly not in Flash based SSDs, but also not in many storage arrays, or for that matter many modern disks that have sectore remapping and various kinds of non-volatile caches. There is a reason why devices tend to have special commands to perform secure erase - depending on the media they might or might not overwrite internally, but at least they do it in a way that actually works for the given media and device configuration.
Quoted 1 line
dm-erase or dm-wipe? dm-discerase?
dm-overwrite?
Quoted 2 lines
But still provide
REQ_OP_SECURE_ERASE
support?
On the one hand that is highly misleading and would warrant a warning (see above), on the other hand discard is purely advisory and can be skipped any time, including by intermediate layers. So I don't think you can actually do what you want without major changes to the whole I/O stack.
Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Vitaly Chikunov
Quoted 3 lines · Christoph Hellwig
Just as a note: the name is a complete misowner, a couple overwrite are not in any way secure deletion. So naming it this way and exposing this as erase is a problem that is going to get back to bite us.
In what way it's not secure deletion?
It's secure deletion by overwriting discarded data instead of leaving it as is.
Quoted 7 lines
Overwriting data does not delete data. Most certainly not in Flash based SSDs, but also not in many storage arrays, or for that matter many modern disks that have sectore remapping and various kinds of non-volatile caches. There is a reason why devices tend to have special commands to perform secure erase - depending on the media they might or might not overwrite internally, but at least they do it in a way that actually works for the given media and device configuration.
I know that. This is why it says "The target does not try to determine if the underlying drive reliably support data overwrites, this decision is solely on the discretion of a user. Please note that not all drivers support this ability."
Quoted 3 lines
Quoted 1 line
dm-erase or dm-wipe? dm-discerase?
dm-overwrite?
These are all good to me.
Quoted 8 lines
Quoted 2 lines
But still provide
REQ_OP_SECURE_ERASEsupport?On the one hand that is highly misleading and would warrant a warning (see above), on the other hand discard is purely advisory and can be skipped any time, including by intermediate layers. So I don't think you can actually do what you want without major changes to the whole I/O stack.
Probably, a concerned user should test his setup to be sure discards reach dm-secdel (after that they go as writes), and data he thinks should be erased is erased.