linux-kernel - Re: [PATCH] CA-392151: fix nfs gup uninitialized iov

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: <20240805141330.GG5334@ZenIV>
Date: Mon, 5 Aug 2024 15:13:30 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Chunjie Zhu <chunjie.zhu@...ud.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] CA-392151: fix nfs gup uninitialized iov_offset defect

On Mon, Aug 05, 2024 at 07:58:14AM +0000, Chunjie Zhu wrote:
>   nfs aio code path, iov_offset is not initialized before used
> 
>   nfs aio function call graph,
>     io_submit
>     aio_read
>     aio_setup_rw
>     import_single_range
>     iov_iter_ubuf           # do not initialize iov_offset

Which compiler it is?  Compound literals initialize *ALL* struct
members.

> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 42bce38a8e87..2121424204c2 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -386,6 +386,7 @@ static inline void iov_iter_ubuf(struct iov_iter *i, unsigned int direction,
>  		.user_backed = true,
>  		.data_source = direction,
>  		.ubuf = buf,
> +		.iov_offset = 0,
>  		.count = count,
>  		.nr_segs = 1
>  	};

NAK.  If you really get an uninitialized value, report it to compiler
authors - it's a bug.  Relevant parts of C99, if you need to quote
it at them:

6.5.2.6[6] The value of the compound literal is that of an unnamed
object initialized by the initializer list. If the compound literal
occurs outside the body of a function, the object has static storage
duration; otherwise, it has automatic storage duration associated with
the enclosing block.

6.5.2.6[7] All the semantic rules and constraints for initializer lists
in 6.7.8 are applicable to compound literals.

6.7.8[21] If there are fewer initializers in a brace-enclosed list than
there are elements or members of an aggregate, or fewer characters in a
string literal used to initialize an array of known size than there are
elements in the array, the remainder of the aggregate shall be initialized
implicitly the same as objects that have static storage duration.

6.7.8[10] If an object that has automatic storage duration is not initialized
explicitly, its value is indeterminate. If an object that has static storage
duration is not initialized explicitly, then:
— if it has pointer type, it is initialized to a null pointer;
— if it has arithmetic type, it is initialized to (positive or unsigned) zero;
— if it is an aggregate, every member is initialized (recursively) according
to these rules;
— if it is a union, the first named member is initialized (recursively) according
to these rules.

Now, it might or might not make sense to spell the initializer for that
member out explicitly on the stylistic grounds, but it is not uninitialized.
Compound literals initialize all (named) members; the only thing left
uninitialized is padding.  If something in your toolchain assumes otherwise,
it needs to be fixed.