* Re: [PATCH alternative 2] block: fix the REQ_OP_SECURE_ERASE handling to not leak erased data
@ 2022-03-18 22:31 kernel test robot
0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-03-18 22:31 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 4218 bytes --]
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220316093855.GC7714@lst.de>
References: <20220316093855.GC7714@lst.de>
TO: Christoph Hellwig <hch@lst.de>
TO: axboe(a)kernel.dk
CC: jaegeuk(a)kernel.org
CC: chao(a)kernel.org
CC: ulf.hansson(a)linaro.org
CC: Adrian Hunter <adrian.hunter@intel.com>
CC: Daeho Jeong <daehojeong@google.com>
CC: Eric Biggers <ebiggers@google.com>
CC: linux-block(a)vger.kernel.org
CC: linux-kernel(a)vger.kernel.org
CC: linux-mmc(a)vger.kernel.org
Hi Christoph,
I love your patch! Perhaps something to improve:
[auto build test WARNING on jaegeuk-f2fs/dev-test]
[also build test WARNING on linux/master linus/master v5.17-rc8]
[cannot apply to axboe-block/for-next next-20220318]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christoph-Hellwig/block-fix-the-REQ_OP_SECURE_ERASE-handling-to-not-leak-erased-data/20220316-174028
base: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: x86_64-randconfig-m001-20220314 (https://download.01.org/0day-ci/archive/20220319/202203190620.wyBlW8VG-lkp(a)intel.com/config)
compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04) 9.4.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
block/blk-lib.c:464 blkdev_issue_secure_erase() warn: should 'len << 9' be a 64 bit type?
Old smatch warnings:
block/blk-lib.c:465 blkdev_issue_secure_erase() warn: should 'len << 9' be a 64 bit type?
vim +464 block/blk-lib.c
04d790607abc51e Christoph Hellwig 2022-03-16 436
04d790607abc51e Christoph Hellwig 2022-03-16 437 int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
04d790607abc51e Christoph Hellwig 2022-03-16 438 sector_t nr_sects, gfp_t gfp)
04d790607abc51e Christoph Hellwig 2022-03-16 439 {
04d790607abc51e Christoph Hellwig 2022-03-16 440 sector_t bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
04d790607abc51e Christoph Hellwig 2022-03-16 441 unsigned int max_sectors =
04d790607abc51e Christoph Hellwig 2022-03-16 442 bdev_get_queue(bdev)->limits.max_discard_sectors;
04d790607abc51e Christoph Hellwig 2022-03-16 443 struct bio *bio = NULL;
04d790607abc51e Christoph Hellwig 2022-03-16 444 struct blk_plug plug;
04d790607abc51e Christoph Hellwig 2022-03-16 445 int ret = 0;
04d790607abc51e Christoph Hellwig 2022-03-16 446
04d790607abc51e Christoph Hellwig 2022-03-16 447 if (max_sectors == 0)
04d790607abc51e Christoph Hellwig 2022-03-16 448 return -EOPNOTSUPP;
04d790607abc51e Christoph Hellwig 2022-03-16 449 if ((sector | nr_sects) & bs_mask)
04d790607abc51e Christoph Hellwig 2022-03-16 450 return -EINVAL;
04d790607abc51e Christoph Hellwig 2022-03-16 451 if (bdev_read_only(bdev))
04d790607abc51e Christoph Hellwig 2022-03-16 452 return -EPERM;
04d790607abc51e Christoph Hellwig 2022-03-16 453
04d790607abc51e Christoph Hellwig 2022-03-16 454 blk_start_plug(&plug);
04d790607abc51e Christoph Hellwig 2022-03-16 455 for (;;) {
04d790607abc51e Christoph Hellwig 2022-03-16 456 unsigned int len = min_t(sector_t, nr_sects, max_sectors);
04d790607abc51e Christoph Hellwig 2022-03-16 457
04d790607abc51e Christoph Hellwig 2022-03-16 458 bio = blk_next_bio(bio, 0, gfp);
04d790607abc51e Christoph Hellwig 2022-03-16 459 bio_set_dev(bio, bdev);
04d790607abc51e Christoph Hellwig 2022-03-16 460 bio->bi_opf = REQ_OP_SECURE_ERASE;
04d790607abc51e Christoph Hellwig 2022-03-16 461 bio->bi_iter.bi_sector = sector;
04d790607abc51e Christoph Hellwig 2022-03-16 462 bio->bi_iter.bi_size = len;
04d790607abc51e Christoph Hellwig 2022-03-16 463
04d790607abc51e Christoph Hellwig 2022-03-16 @464 sector += len << SECTOR_SHIFT;
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH alternative 2] block: fix the REQ_OP_SECURE_ERASE handling to not leak erased data
2022-03-18 9:11 ` Christoph Hellwig
@ 2022-03-18 10:36 ` Ulf Hansson
0 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2022-03-18 10:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: axboe, jaegeuk, chao, Adrian Hunter, Daeho Jeong, Eric Biggers,
linux-block, linux-kernel, linux-mmc
On Fri, 18 Mar 2022 at 10:11, Christoph Hellwig <hch@lst.de> wrote:
>
> On Thu, Mar 17, 2022 at 10:44:01AM +0100, Ulf Hansson wrote:
> > Stating that it can't work is probably not a correct statement.
> > Certainly it can, but it depends on how "secure" (or clever) the
> > implementation of the FTL is in the flash media. I mean, nothing
> > prevents the FTL from doing a real erase on erase block level and
> > simply let the "secure erase" request wait on that operation to be
> > completed.
>
> Well, that assumes it can find all the previous copied of the data.
> Having worked with various higher end SSDs FTLs I know they can't,
> so if an eMMC device could that would very much surpise me given
> the overhead.
An eMMC is no different from an SSD in this regard, so you are most
definitely correct. BTW, I was one of those guys working with FTLs
myself, but it was a long time ago, when NAND/NOR flashes were less
complicated to manage.
Anyway, to really make things work, one would need some additional low
level partitioning - or commands to tag the data for special
purposes. eMMCs do have some support for things like this, but whether
it actually works to serve this particular use case (secure erase), I
really can't tell.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH alternative 2] block: fix the REQ_OP_SECURE_ERASE handling to not leak erased data
2022-03-17 9:44 ` Ulf Hansson
@ 2022-03-18 9:11 ` Christoph Hellwig
2022-03-18 10:36 ` Ulf Hansson
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2022-03-18 9:11 UTC (permalink / raw)
To: Ulf Hansson
Cc: Christoph Hellwig, axboe, jaegeuk, chao, Adrian Hunter,
Daeho Jeong, Eric Biggers, linux-block, linux-kernel, linux-mmc
On Thu, Mar 17, 2022 at 10:44:01AM +0100, Ulf Hansson wrote:
> Stating that it can't work is probably not a correct statement.
> Certainly it can, but it depends on how "secure" (or clever) the
> implementation of the FTL is in the flash media. I mean, nothing
> prevents the FTL from doing a real erase on erase block level and
> simply let the "secure erase" request wait on that operation to be
> completed.
Well, that assumes it can find all the previous copied of the data.
Having worked with various higher end SSDs FTLs I know they can't,
so if an eMMC device could that would very much surpise me given
the overhead.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH alternative 2] block: fix the REQ_OP_SECURE_ERASE handling to not leak erased data
2022-03-16 9:38 ` [PATCH alternative 2] block: fix the REQ_OP_SECURE_ERASE handling to not leak erased data Christoph Hellwig
@ 2022-03-17 9:44 ` Ulf Hansson
2022-03-18 9:11 ` Christoph Hellwig
0 siblings, 1 reply; 5+ messages in thread
From: Ulf Hansson @ 2022-03-17 9:44 UTC (permalink / raw)
To: Christoph Hellwig
Cc: axboe, jaegeuk, chao, Adrian Hunter, Daeho Jeong, Eric Biggers,
linux-block, linux-kernel, linux-mmc
On Wed, 16 Mar 2022 at 10:38, Christoph Hellwig <hch@lst.de> wrote:
>
> The support for this "secure erase" is completely broken, given that
> the blk-lib code aligns it to the discard granularity and alignment
> and thus skips parts of the two be discarded area, leaking plenty of
> securely erased data. Fix this by adding a new blkdev_secure_erase
> helper instead.
>
> Note that even if with these rounding errors fixed, a LBA based
> "secure erase" can't actually work on flash media. As flash media
> requires erase cycles before writing instead of overwrites there
> usually will be copied of this data left somewhere on the media.
Stating that it can't work is probably not a correct statement.
Certainly it can, but it depends on how "secure" (or clever) the
implementation of the FTL is in the flash media. I mean, nothing
prevents the FTL from doing a real erase on erase block level and
simply let the "secure erase" request wait on that operation to be
completed.
It looks like the use-cases for "secure erase" are just trying with a
"best effort" in mind. There are no guarantees that the data is really
wiped out from flash, but if it can, it's better than keeping it
around. I guess the real problem comes when the use-case actually
believes that the data is guaranteed to be wiped out, while it may
not.
I really don't have a strong opinion on what way we want to go with
this. Both alternative 1 and alternative 2 work for me, so I leave the
call to you and others.
Kind regards
Uffe
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> block/blk-lib.c | 55 +++++++++++++++++++++++------
> block/ioctl.c | 43 +++++++++++++++++-----
> drivers/block/xen-blkback/blkback.c | 15 ++++----
> fs/f2fs/file.c | 9 ++---
> include/linux/blkdev.h | 4 +--
> 5 files changed, 95 insertions(+), 31 deletions(-)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 9f09beadcbe30..5fc2c0bf5c940 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -29,7 +29,7 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> {
> struct request_queue *q = bdev_get_queue(bdev);
> struct bio *bio = *biop;
> - unsigned int op;
> + unsigned int op = REQ_OP_DISCARD;
> sector_t bs_mask, part_offset = 0;
>
> if (!q)
> @@ -38,15 +38,8 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> if (bdev_read_only(bdev))
> return -EPERM;
>
> - if (flags & BLKDEV_DISCARD_SECURE) {
> - if (!blk_queue_secure_erase(q))
> - return -EOPNOTSUPP;
> - op = REQ_OP_SECURE_ERASE;
> - } else {
> - if (!blk_queue_discard(q))
> - return -EOPNOTSUPP;
> - op = REQ_OP_DISCARD;
> - }
> + if (!blk_queue_discard(q))
> + return -EOPNOTSUPP;
>
> /* In case the discard granularity isn't set by buggy device driver */
> if (WARN_ON_ONCE(!q->limits.discard_granularity)) {
> @@ -440,3 +433,45 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
> return ret;
> }
> EXPORT_SYMBOL(blkdev_issue_zeroout);
> +
> +int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
> + sector_t nr_sects, gfp_t gfp)
> +{
> + sector_t bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
> + unsigned int max_sectors =
> + bdev_get_queue(bdev)->limits.max_discard_sectors;
> + struct bio *bio = NULL;
> + struct blk_plug plug;
> + int ret = 0;
> +
> + if (max_sectors == 0)
> + return -EOPNOTSUPP;
> + if ((sector | nr_sects) & bs_mask)
> + return -EINVAL;
> + if (bdev_read_only(bdev))
> + return -EPERM;
> +
> + blk_start_plug(&plug);
> + for (;;) {
> + unsigned int len = min_t(sector_t, nr_sects, max_sectors);
> +
> + bio = blk_next_bio(bio, 0, gfp);
> + bio_set_dev(bio, bdev);
> + bio->bi_opf = REQ_OP_SECURE_ERASE;
> + bio->bi_iter.bi_sector = sector;
> + bio->bi_iter.bi_size = len;
> +
> + sector += len << SECTOR_SHIFT;
> + nr_sects -= len << SECTOR_SHIFT;
> + if (!nr_sects) {
> + ret = submit_bio_wait(bio);
> + bio_put(bio);
> + break;
> + }
> + cond_resched();
> + }
> + blk_finish_plug(&plug);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(blkdev_issue_secure_erase);
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 4a86340133e46..0821142f921d7 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -83,7 +83,7 @@ static int compat_blkpg_ioctl(struct block_device *bdev,
> #endif
>
> static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode,
> - unsigned long arg, unsigned long flags)
> + unsigned long arg)
> {
> uint64_t range[2];
> uint64_t start, len;
> @@ -115,15 +115,43 @@ static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode,
> err = truncate_bdev_range(bdev, mode, start, start + len - 1);
> if (err)
> goto fail;
> -
> - err = blkdev_issue_discard(bdev, start >> 9, len >> 9,
> - GFP_KERNEL, flags);
> -
> + err = blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
> fail:
> filemap_invalidate_unlock(inode->i_mapping);
> return err;
> }
>
> +static int blk_ioctl_secure_erase(struct block_device *bdev, fmode_t mode,
> + void __user *argp)
> +{
> + uint64_t start, len;
> + uint64_t range[2];
> + int err;
> +
> + if (!(mode & FMODE_WRITE))
> + return -EBADF;
> + if (!blk_queue_discard(bdev_get_queue(bdev)))
> + return -EOPNOTSUPP;
> + if (copy_from_user(range, argp, sizeof(range)))
> + return -EFAULT;
> +
> + start = range[0];
> + len = range[1];
> + if ((start & 511) || (len & 511))
> + return -EINVAL;
> + if (start + len > bdev_nr_bytes(bdev))
> + return -EINVAL;
> +
> + filemap_invalidate_lock(bdev->bd_inode->i_mapping);
> + err = truncate_bdev_range(bdev, mode, start, start + len - 1);
> + if (!err)
> + err = blkdev_issue_secure_erase(bdev, start >> 9, len >> 9,
> + GFP_KERNEL);
> + filemap_invalidate_unlock(bdev->bd_inode->i_mapping);
> + return err;
> +}
> +
> +
> static int blk_ioctl_zeroout(struct block_device *bdev, fmode_t mode,
> unsigned long arg)
> {
> @@ -451,10 +479,9 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
> case BLKROSET:
> return blkdev_roset(bdev, mode, cmd, arg);
> case BLKDISCARD:
> - return blk_ioctl_discard(bdev, mode, arg, 0);
> + return blk_ioctl_discard(bdev, mode, arg);
> case BLKSECDISCARD:
> - return blk_ioctl_discard(bdev, mode, arg,
> - BLKDEV_DISCARD_SECURE);
> + return blk_ioctl_secure_erase(bdev, mode, argp);
> case BLKZEROOUT:
> return blk_ioctl_zeroout(bdev, mode, arg);
> case BLKGETDISKSEQ:
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index 14e452896d04c..12f741068bcdf 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -970,7 +970,6 @@ static int dispatch_discard_io(struct xen_blkif_ring *ring,
> int status = BLKIF_RSP_OKAY;
> struct xen_blkif *blkif = ring->blkif;
> struct block_device *bdev = blkif->vbd.bdev;
> - unsigned long secure;
> struct phys_req preq;
>
> xen_blkif_get(blkif);
> @@ -987,13 +986,15 @@ static int dispatch_discard_io(struct xen_blkif_ring *ring,
> }
> ring->st_ds_req++;
>
> - secure = (blkif->vbd.discard_secure &&
> - (req->u.discard.flag & BLKIF_DISCARD_SECURE)) ?
> - BLKDEV_DISCARD_SECURE : 0;
> + if (blkif->vbd.discard_secure &&
> + (req->u.discard.flag & BLKIF_DISCARD_SECURE))
> + err = blkdev_issue_secure_erase(bdev,
> + req->u.discard.sector_number,
> + req->u.discard.nr_sectors, GFP_KERNEL);
> + else
> + err = blkdev_issue_discard(bdev, req->u.discard.sector_number,
> + req->u.discard.nr_sectors, GFP_KERNEL, 0);
>
> - err = blkdev_issue_discard(bdev, req->u.discard.sector_number,
> - req->u.discard.nr_sectors,
> - GFP_KERNEL, secure);
> fail_response:
> if (err == -EOPNOTSUPP) {
> pr_debug("discard op failed, not supported\n");
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 3c98ef6af97d1..a83548ad7171f 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3691,10 +3691,11 @@ static int f2fs_secure_erase(struct block_device *bdev, struct inode *inode,
> if (!q)
> return -ENXIO;
>
> - if (flags & F2FS_TRIM_FILE_DISCARD)
> - ret = blkdev_issue_discard(bdev, sector, nr_sects, GFP_NOFS,
> - blk_queue_secure_erase(q) ?
> - BLKDEV_DISCARD_SECURE : 0);
> + if ((flags & F2FS_TRIM_FILE_DISCARD) && blk_queue_secure_erase(q))
> + ret = blkdev_issue_secure_erase(bdev, sector, nr_sects,
> + GFP_NOFS);
> + else if (flags & F2FS_TRIM_FILE_DISCARD)
> + ret = blkdev_issue_discard(bdev, sector, nr_sects, GFP_NOFS, 0);
>
> if (!ret && (flags & F2FS_TRIM_FILE_ZEROOUT)) {
> if (IS_ENCRYPTED(inode))
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 16b47035e4b06..6cfc60090b119 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -846,13 +846,13 @@ extern void blk_io_schedule(void);
> extern int blkdev_issue_write_same(struct block_device *bdev, sector_t sector,
> sector_t nr_sects, gfp_t gfp_mask, struct page *page);
>
> -#define BLKDEV_DISCARD_SECURE (1 << 0) /* issue a secure erase */
> -
> extern int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> sector_t nr_sects, gfp_t gfp_mask, unsigned long flags);
> extern int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
> sector_t nr_sects, gfp_t gfp_mask, int flags,
> struct bio **biop);
> +int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
> + sector_t nr_sects, gfp_t gfp);
>
> #define BLKDEV_ZERO_NOUNMAP (1 << 0) /* do not free blocks */
> #define BLKDEV_ZERO_NOFALLBACK (1 << 1) /* don't write explicit zeroes */
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH alternative 2] block: fix the REQ_OP_SECURE_ERASE handling to not leak erased data
2022-03-16 9:37 security issue: data exposure when using block layer secure erase Christoph Hellwig
@ 2022-03-16 9:38 ` Christoph Hellwig
2022-03-17 9:44 ` Ulf Hansson
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2022-03-16 9:38 UTC (permalink / raw)
To: axboe
Cc: jaegeuk, chao, ulf.hansson, Adrian Hunter, Daeho Jeong,
Eric Biggers, linux-block, linux-kernel, linux-mmc
The support for this "secure erase" is completely broken, given that
the blk-lib code aligns it to the discard granularity and alignment
and thus skips parts of the two be discarded area, leaking plenty of
securely erased data. Fix this by adding a new blkdev_secure_erase
helper instead.
Note that even if with these rounding errors fixed, a LBA based
"secure erase" can't actually work on flash media. As flash media
requires erase cycles before writing instead of overwrites there
usually will be copied of this data left somewhere on the media.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-lib.c | 55 +++++++++++++++++++++++------
block/ioctl.c | 43 +++++++++++++++++-----
drivers/block/xen-blkback/blkback.c | 15 ++++----
fs/f2fs/file.c | 9 ++---
include/linux/blkdev.h | 4 +--
5 files changed, 95 insertions(+), 31 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 9f09beadcbe30..5fc2c0bf5c940 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -29,7 +29,7 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
{
struct request_queue *q = bdev_get_queue(bdev);
struct bio *bio = *biop;
- unsigned int op;
+ unsigned int op = REQ_OP_DISCARD;
sector_t bs_mask, part_offset = 0;
if (!q)
@@ -38,15 +38,8 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
if (bdev_read_only(bdev))
return -EPERM;
- if (flags & BLKDEV_DISCARD_SECURE) {
- if (!blk_queue_secure_erase(q))
- return -EOPNOTSUPP;
- op = REQ_OP_SECURE_ERASE;
- } else {
- if (!blk_queue_discard(q))
- return -EOPNOTSUPP;
- op = REQ_OP_DISCARD;
- }
+ if (!blk_queue_discard(q))
+ return -EOPNOTSUPP;
/* In case the discard granularity isn't set by buggy device driver */
if (WARN_ON_ONCE(!q->limits.discard_granularity)) {
@@ -440,3 +433,45 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
return ret;
}
EXPORT_SYMBOL(blkdev_issue_zeroout);
+
+int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
+ sector_t nr_sects, gfp_t gfp)
+{
+ sector_t bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
+ unsigned int max_sectors =
+ bdev_get_queue(bdev)->limits.max_discard_sectors;
+ struct bio *bio = NULL;
+ struct blk_plug plug;
+ int ret = 0;
+
+ if (max_sectors == 0)
+ return -EOPNOTSUPP;
+ if ((sector | nr_sects) & bs_mask)
+ return -EINVAL;
+ if (bdev_read_only(bdev))
+ return -EPERM;
+
+ blk_start_plug(&plug);
+ for (;;) {
+ unsigned int len = min_t(sector_t, nr_sects, max_sectors);
+
+ bio = blk_next_bio(bio, 0, gfp);
+ bio_set_dev(bio, bdev);
+ bio->bi_opf = REQ_OP_SECURE_ERASE;
+ bio->bi_iter.bi_sector = sector;
+ bio->bi_iter.bi_size = len;
+
+ sector += len << SECTOR_SHIFT;
+ nr_sects -= len << SECTOR_SHIFT;
+ if (!nr_sects) {
+ ret = submit_bio_wait(bio);
+ bio_put(bio);
+ break;
+ }
+ cond_resched();
+ }
+ blk_finish_plug(&plug);
+
+ return ret;
+}
+EXPORT_SYMBOL(blkdev_issue_secure_erase);
diff --git a/block/ioctl.c b/block/ioctl.c
index 4a86340133e46..0821142f921d7 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -83,7 +83,7 @@ static int compat_blkpg_ioctl(struct block_device *bdev,
#endif
static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode,
- unsigned long arg, unsigned long flags)
+ unsigned long arg)
{
uint64_t range[2];
uint64_t start, len;
@@ -115,15 +115,43 @@ static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode,
err = truncate_bdev_range(bdev, mode, start, start + len - 1);
if (err)
goto fail;
-
- err = blkdev_issue_discard(bdev, start >> 9, len >> 9,
- GFP_KERNEL, flags);
-
+ err = blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL, 0);
fail:
filemap_invalidate_unlock(inode->i_mapping);
return err;
}
+static int blk_ioctl_secure_erase(struct block_device *bdev, fmode_t mode,
+ void __user *argp)
+{
+ uint64_t start, len;
+ uint64_t range[2];
+ int err;
+
+ if (!(mode & FMODE_WRITE))
+ return -EBADF;
+ if (!blk_queue_discard(bdev_get_queue(bdev)))
+ return -EOPNOTSUPP;
+ if (copy_from_user(range, argp, sizeof(range)))
+ return -EFAULT;
+
+ start = range[0];
+ len = range[1];
+ if ((start & 511) || (len & 511))
+ return -EINVAL;
+ if (start + len > bdev_nr_bytes(bdev))
+ return -EINVAL;
+
+ filemap_invalidate_lock(bdev->bd_inode->i_mapping);
+ err = truncate_bdev_range(bdev, mode, start, start + len - 1);
+ if (!err)
+ err = blkdev_issue_secure_erase(bdev, start >> 9, len >> 9,
+ GFP_KERNEL);
+ filemap_invalidate_unlock(bdev->bd_inode->i_mapping);
+ return err;
+}
+
+
static int blk_ioctl_zeroout(struct block_device *bdev, fmode_t mode,
unsigned long arg)
{
@@ -451,10 +479,9 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
case BLKROSET:
return blkdev_roset(bdev, mode, cmd, arg);
case BLKDISCARD:
- return blk_ioctl_discard(bdev, mode, arg, 0);
+ return blk_ioctl_discard(bdev, mode, arg);
case BLKSECDISCARD:
- return blk_ioctl_discard(bdev, mode, arg,
- BLKDEV_DISCARD_SECURE);
+ return blk_ioctl_secure_erase(bdev, mode, argp);
case BLKZEROOUT:
return blk_ioctl_zeroout(bdev, mode, arg);
case BLKGETDISKSEQ:
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 14e452896d04c..12f741068bcdf 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -970,7 +970,6 @@ static int dispatch_discard_io(struct xen_blkif_ring *ring,
int status = BLKIF_RSP_OKAY;
struct xen_blkif *blkif = ring->blkif;
struct block_device *bdev = blkif->vbd.bdev;
- unsigned long secure;
struct phys_req preq;
xen_blkif_get(blkif);
@@ -987,13 +986,15 @@ static int dispatch_discard_io(struct xen_blkif_ring *ring,
}
ring->st_ds_req++;
- secure = (blkif->vbd.discard_secure &&
- (req->u.discard.flag & BLKIF_DISCARD_SECURE)) ?
- BLKDEV_DISCARD_SECURE : 0;
+ if (blkif->vbd.discard_secure &&
+ (req->u.discard.flag & BLKIF_DISCARD_SECURE))
+ err = blkdev_issue_secure_erase(bdev,
+ req->u.discard.sector_number,
+ req->u.discard.nr_sectors, GFP_KERNEL);
+ else
+ err = blkdev_issue_discard(bdev, req->u.discard.sector_number,
+ req->u.discard.nr_sectors, GFP_KERNEL, 0);
- err = blkdev_issue_discard(bdev, req->u.discard.sector_number,
- req->u.discard.nr_sectors,
- GFP_KERNEL, secure);
fail_response:
if (err == -EOPNOTSUPP) {
pr_debug("discard op failed, not supported\n");
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 3c98ef6af97d1..a83548ad7171f 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3691,10 +3691,11 @@ static int f2fs_secure_erase(struct block_device *bdev, struct inode *inode,
if (!q)
return -ENXIO;
- if (flags & F2FS_TRIM_FILE_DISCARD)
- ret = blkdev_issue_discard(bdev, sector, nr_sects, GFP_NOFS,
- blk_queue_secure_erase(q) ?
- BLKDEV_DISCARD_SECURE : 0);
+ if ((flags & F2FS_TRIM_FILE_DISCARD) && blk_queue_secure_erase(q))
+ ret = blkdev_issue_secure_erase(bdev, sector, nr_sects,
+ GFP_NOFS);
+ else if (flags & F2FS_TRIM_FILE_DISCARD)
+ ret = blkdev_issue_discard(bdev, sector, nr_sects, GFP_NOFS, 0);
if (!ret && (flags & F2FS_TRIM_FILE_ZEROOUT)) {
if (IS_ENCRYPTED(inode))
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 16b47035e4b06..6cfc60090b119 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -846,13 +846,13 @@ extern void blk_io_schedule(void);
extern int blkdev_issue_write_same(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, struct page *page);
-#define BLKDEV_DISCARD_SECURE (1 << 0) /* issue a secure erase */
-
extern int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, unsigned long flags);
extern int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, int flags,
struct bio **biop);
+int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
+ sector_t nr_sects, gfp_t gfp);
#define BLKDEV_ZERO_NOUNMAP (1 << 0) /* do not free blocks */
#define BLKDEV_ZERO_NOFALLBACK (1 << 1) /* don't write explicit zeroes */
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-18 22:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-18 22:31 [PATCH alternative 2] block: fix the REQ_OP_SECURE_ERASE handling to not leak erased data kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2022-03-16 9:37 security issue: data exposure when using block layer secure erase Christoph Hellwig
2022-03-16 9:38 ` [PATCH alternative 2] block: fix the REQ_OP_SECURE_ERASE handling to not leak erased data Christoph Hellwig
2022-03-17 9:44 ` Ulf Hansson
2022-03-18 9:11 ` Christoph Hellwig
2022-03-18 10:36 ` Ulf Hansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.