Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Wed, 16 Mar 2022 10:37:40 +0100
From:   Christoph Hellwig <hch@lst.de>
To:     axboe@kernel.dk
Cc:     jaegeuk@kernel.org, chao@kernel.org, ulf.hansson@linaro.org,
        Adrian Hunter <adrian.hunter@intel.com>,
        Daeho Jeong <daehojeong@google.com>,
        Eric Biggers <ebiggers@google.com>,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-mmc@vger.kernel.org
Subject: security issue: data exposure when using block layer secure erase
Message-ID: <20220316093740.GA7714@lst.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.17 (2007-11-01)
Precedence: bulk

Hi all,

while staring at the block layer code I found what I think is a major
security issue with the use of REQ_OP_SECURE_ERASE.

The issue is not about the actual protocol implementation, which only
exists for eMMC [1], but about we handle issuing the operation in the
block layer.  That is done through __blkdev_issue_discard, which
takes various parameters into account to align the issue discard
request to what the hardware prefers.  Which is perfectly fine for
discard as an advisory operation, but deadly for an operation that
wants to make data inaccessible.  The problem has existed ever since
secure erase support was added to the kernel with commit
8d57a98ccd0b ("block: add secure discard"), which added secure erase
support as a REQ_SECURE flag to the discard operation.

The ioctl added there also as the only users for a long time, until f2fs
added a second (really strange) user that uses secure erase if offered by 
the device but otherwise plain old discard: 9af846486d78
("f2fs: add F2FS_IOC_SEC_TRIM_FILE ioctl") which seems to treat the
secure discard as nice to have but actually is fine with data leaks
from the use of discard or an incorrect implementation of secure erase.

My preference would be to just remove this ill designed feature entirely.
The alternative 1 in this thead does just that.  Alternative 2 tries to
fix it instead, but I haven't bee nable to get any interested party to
actually test in more than three eeks, suggesting we're better off
removing the code.

[1] which is rather dubious as well, as sector based secure erase in
flash based media can't really work due to the lack of in-place write
support.  At best it is the equivalent for a Write Same or Write Zeroes
command without deterministic data on the next read.