linux-kernel - Re: [PATCH RFC 4/4] md/raid0: Atomic write support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <21f19b4b-4b83-4ca2-a93b-0a433741fd26@oracle.com>
Date: Thu, 12 Sep 2024 16:38:37 +0100
From: John Garry <john.g.garry@...cle.com>
To: Christoph Hellwig <hch@....de>
Cc: axboe@...nel.dk, song@...nel.org, yukuai3@...wei.com, kbusch@...nel.org,
        sagi@...mberg.me, James.Bottomley@...senPartnership.com,
        martin.petersen@...cle.com, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-raid@...r.kernel.org,
        linux-nvme@...ts.infradead.org, linux-scsi@...r.kernel.org
Subject: Re: [PATCH RFC 4/4] md/raid0: Atomic write support

On 12/09/2024 16:10, Christoph Hellwig wrote:
> On Thu, Sep 12, 2024 at 03:48:09PM +0100, John Garry wrote:
>>
>> I actually now think that I should change bio_split() to return NULL for
>> splitting a REQ_ATOMIC, like what do for ZONE_APPEND - calling bio_split()
>> like this is a common pattern in md RAID personalities. However, none of
>> the md RAID code check for a NULL split, which they really should, so I can
>> make that change also.
> 
> bio_split is a bit of a mess - even NULL isn't very good at returning
> what caused it to fail.  Maybe a switch to ERR_PTR and an audit of
> all callers might be a good idea.
> 

So for bio_split() I guess that we would change as follows:

--->8----


diff --git a/block/bio.c b/block/bio.c
index c4053d49679a..36ddf458753f 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1671,11 +1671,11 @@ struct bio *bio_split(struct bio *bio, int sectors,

  	/* Zone append commands cannot be split */
  	if (WARN_ON_ONCE(bio_op(bio) == REQ_OP_ZONE_APPEND))
-		return NULL;
+		return ERR_PTR(-EOPNOTSUPP);

  	split = bio_alloc_clone(bio->bi_bdev, bio, gfp, bs);
  	if (!split)
-		return NULL;
+		return ERR_PTR(-ENOMEM);

  	split->bi_iter.bi_size = sectors << 9;


---8<----

And then fix up the callers to use IS_ERR().

Or also change bio_alloc_clone() to return an ERR_PTR()? I don't see 
much point in that, as we will only ever return ENOMEM (from the 
callees) anyway.