diff mbox series

[v3,5/6] blk-mq: return actual keyslot error in blk_insert_cloned_request()

Message ID 20230315183907.53675-6-ebiggers@kernel.org (mailing list archive)
State New, archived
Headers show
Series Fix blk-crypto keyslot race condition | expand

Commit Message

Eric Biggers March 15, 2023, 6:39 p.m. UTC 
From: Eric Biggers <ebiggers@google.com>

To avoid hiding information, pass on the error code from
blk_crypto_rq_get_keyslot() instead of always using BLK_STS_IOERR.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 block/blk-mq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Jens Axboe March 15, 2023, 6:50 p.m. UTC | #1 
On 3/15/23 12:39 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> To avoid hiding information, pass on the error code from
> blk_crypto_rq_get_keyslot() instead of always using BLK_STS_IOERR.

Maybe just fold this with the previous patch?
Eric Biggers March 15, 2023, 6:54 p.m. UTC | #2 
On Wed, Mar 15, 2023 at 12:50:45PM -0600, Jens Axboe wrote:
> On 3/15/23 12:39 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > To avoid hiding information, pass on the error code from
> > blk_crypto_rq_get_keyslot() instead of always using BLK_STS_IOERR.
> 
> Maybe just fold this with the previous patch?

I'd prefer to keep the behavior change separate from the cleanup.

- Eric
Jens Axboe March 15, 2023, 6:55 p.m. UTC | #3 
On 3/15/23 12:54 PM, Eric Biggers wrote:
> On Wed, Mar 15, 2023 at 12:50:45PM -0600, Jens Axboe wrote:
>> On 3/15/23 12:39 PM, Eric Biggers wrote:
>>> From: Eric Biggers <ebiggers@google.com>
>>>
>>> To avoid hiding information, pass on the error code from
>>> blk_crypto_rq_get_keyslot() instead of always using BLK_STS_IOERR.
>>
>> Maybe just fold this with the previous patch?
> 
> I'd prefer to keep the behavior change separate from the cleanup.

OK fair enough, not a big deal to me. Series looks fine as far as
I'm concerned. Not loving the extra additions in the completion path,
but I suppose there's no way around that.
Eric Biggers March 15, 2023, 7:04 p.m. UTC | #4 
On Wed, Mar 15, 2023 at 12:55:31PM -0600, Jens Axboe wrote:
> On 3/15/23 12:54 PM, Eric Biggers wrote:
> > On Wed, Mar 15, 2023 at 12:50:45PM -0600, Jens Axboe wrote:
> >> On 3/15/23 12:39 PM, Eric Biggers wrote:
> >>> From: Eric Biggers <ebiggers@google.com>
> >>>
> >>> To avoid hiding information, pass on the error code from
> >>> blk_crypto_rq_get_keyslot() instead of always using BLK_STS_IOERR.
> >>
> >> Maybe just fold this with the previous patch?
> > 
> > I'd prefer to keep the behavior change separate from the cleanup.
> 
> OK fair enough, not a big deal to me. Series looks fine as far as
> I'm concerned. Not loving the extra additions in the completion path,
> but I suppose there's no way around that.

Well, my first patch didn't have that:

https://lore.kernel.org/linux-block/20230226203816.207449-1-ebiggers@kernel.org

But it didn't fix the keyslot refcounting work as expected, which Nathan didn't
like (and I agree with that).

- Eric
Christoph Hellwig March 16, 2023, 3:15 p.m. UTC | #5 
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 5e819de2f5e70..a875b1cdff9b5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3049,8 +3049,9 @@  blk_status_t blk_insert_cloned_request(struct request *rq)
 	if (q->disk && should_fail_request(q->disk->part0, blk_rq_bytes(rq)))
 		return BLK_STS_IOERR;
 
-	if (blk_crypto_rq_get_keyslot(rq))
-		return BLK_STS_IOERR;
+	ret = blk_crypto_rq_get_keyslot(rq);
+	if (ret != BLK_STS_OK)
+		return ret;
 
 	blk_account_io_start(rq);