Re: [bug report] block: bio-integrity: directly map user buffers

[Keith Busch <kbusch@kernel.org>]

On Tue, Dec 05, 2023 at 12:35:30PM +0300, Dan Carpenter wrote:
> Hello Keith Busch,
> 
> The patch 492c5d455969: "block: bio-integrity: directly map user
> buffers" from Nov 30, 2023 (linux-next), leads to the following
> Smatch static checker warning:
> 
> 	block/bio-integrity.c:350 bio_integrity_map_user()
> 	error: uninitialized symbol 'offset'.
> 
> block/bio-integrity.c
>     340                 if (!bvec)
>     341                         return -ENOMEM;
>     342                 pages = NULL;
>     343         }
>     344 
>     345         copy = !iov_iter_is_aligned(&iter, align, align);
>     346         ret = iov_iter_extract_pages(&iter, &pages, bytes, nr_vecs, 0, &offset);
> 
> Smatch is concerned about the first "return 0;" if bytes or iter.count
> is zero.  In that situation then offset is uninitialized.
>
>     347         if (unlikely(ret < 0))
>     348                 goto free_bvec;
>     349 
> --> 350         nr_bvecs = bvec_from_pages(bvec, pages, nr_vecs, bytes, offset);
>                                                                         ^^^^^^

Thanks for the report! I don't think there's any scenario where someone
would purposefully request a 0 length metadata mapping, so I'll have it
return EINVAL for that condition.

But ... bvec_from_pages() only reads 'offset' if nr_vecs > 0. nr_vecs
would have to be 0 in this case, so it's not really accessing an
uninitialized variable. Everything in fact appears to "work" if you do
request 0 length, though again, I don't think there's a legit reason to
ever do that.