[PATCH] CA-392151: fix nfs gup uninitialized iov_offset defect

[PATCH] CA-392151: fix nfs gup uninitialized iov_offset defect

[Chunjie Zhu]

  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
    call_read_iter
    nfs_file_read
    nfs_file_direct_read
    nfs_direct_read_schedule_iovec
    iov_iter_get_pages_alloc2
    __iov_iter_get_pages_alloc
    first_iovec_segment     # iov_offset is used, not initialized

Signed-off-by: Chunjie Zhu <chunjie.zhu@cloud.com>
---
 include/linux/uio.h | 1 +
 1 file changed, 1 insertion(+)

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
 	};
-- 
2.34.1

Re: [PATCH] CA-392151: fix nfs gup uninitialized iov_offset defect

[Al Viro]

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.