linux-kernel - Re: Explanation on Uninitialized Variable bio in blk_rq_prep

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] [day] [month] [year] [list]

Message-ID: <ZwN2uFsOxO3jL-EY@infradead.org>
Date: Sun, 6 Oct 2024 22:50:48 -0700
From: Christoph Hellwig <hch@...radead.org>
To: Suraj Sonawane <surajsonawane0215@...il.com>
Cc: Keith Busch <kbusch@...nel.org>, hch@...radead.org, axboe@...nel.dk,
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: Explanation on Uninitialized Variable bio in blk_rq_prep_clone

On Sun, Oct 06, 2024 at 12:33:58PM +0530, Suraj Sonawane wrote:
> > If it fails, then bio is initialized to NULL.
> You're correct, bio_alloc_clone returns NULL if it fails, so there’s no
> uninitialized bio after that. My initial explanation wasn’t fully accurate,
> but initializing bio to NULL is just a safety measure for any unexpected
> issues later on. Or i am just trying to solve this issue by smatch tool:
> block/blk-mq.c:3199 blk_rq_prep_clone() error: uninitialized symbol 'bio'.

Please do this kind of research and clearly state it in the
commit log.

Now the actual useful cleanup here would be to:

 - move the bio_put to the bio_ctr error handling, which is the only
   case where it can happen
 - move the bio variable into the __rq_for_each_bio scope, which
   also removed the need to zero it at the end of the loop

That makes the easier to reason about, which should make whatever
static checker you have happy, and also allows humans to read it more
nicely.