[PATCH RFC netfs] Fix uninitialized variable in netfs_retry_read_subrequests()

[PATCH RFC netfs] Fix uninitialized variable in netfs_retry_read_subrequests()

[Paul E. McKenney]

Hello!

This should actually be considered more of a bug report than a patch.

Clang 18.1.8 (but not GCC 11.5.0) complains that the "subreq" local
variable can be used uninitialized in netfs_retry_read_subrequests(),
just after the abandon_after label.  This function is unusual in having
three instances of this local variable.  The third and last one is clearly
erroneous because there is a branch out of the enclosing do-while loop
to the end of this function, and it looks like the intent is that the
code at the end of this function be using the same value of the "subreq"
local variable as is used within that do-while loop.

Therefore, take the obvious (if potentially quite misguided) approach
of removing the third declaration of "subreq", instead simply setting
it to NULL.

Not-yet-signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: <netfs@lists.linux.dev>
Cc: <linux-fsdevel@vger.kernel.org>

diff --git a/fs/netfs/read_retry.c b/fs/netfs/read_retry.c
index 8ca0558570c14..eba684b408df1 100644
--- a/fs/netfs/read_retry.c
+++ b/fs/netfs/read_retry.c
@@ -72,12 +72,14 @@ static void netfs_retry_read_subrequests(struct netfs_io_request *rreq)
 	next = stream->subrequests.next;
 
 	do {
-		struct netfs_io_subrequest *subreq = NULL, *from, *to, *tmp;
+		struct netfs_io_subrequest *from, *to, *tmp;
 		struct iov_iter source;
 		unsigned long long start, len;
 		size_t part;
 		bool boundary = false;
 
+		subreq = NULL;
+
 		/* Go through the subreqs and find the next span of contiguous
 		 * buffer that we then rejig (cifs, for example, needs the
 		 * rsize renegotiating) and reissue.

Re: [PATCH RFC netfs] Fix uninitialized variable in netfs_retry_read_subrequests()

[David Howells]

Paul E. McKenney <paulmck@kernel.org> wrote:

> This should actually be considered more of a bug report than a patch.
> 
> Clang 18.1.8 (but not GCC 11.5.0) complains that the "subreq" local
> variable can be used uninitialized in netfs_retry_read_subrequests(),
> just after the abandon_after label.  This function is unusual in having
> three instances of this local variable.  The third and last one is clearly
> erroneous because there is a branch out of the enclosing do-while loop
> to the end of this function, and it looks like the intent is that the
> code at the end of this function be using the same value of the "subreq"
> local variable as is used within that do-while loop.
> 
> Therefore, take the obvious (if potentially quite misguided) approach
> of removing the third declaration of "subreq", instead simply setting
> it to NULL.

I think you're looking at the old version of my netfs-writeback branch that's
residing in Christian's vfs.netfs branch.  I've posted a new version of my
branch[1] without this problem and am hoping for Christian to update the
branch[2] so that Stephen can pull it into linux-next.

David

[1] https://lore.kernel.org/linux-fsdevel/20241216204124.3752367-1-dhowells@redhat.com/T/#t

[2] And hoping he'll remember to drop "[PATCH v5 26/32] Display waited-on page
index after 1min of waiting" for me.  I forgot to remove that debugging patch.

Re: [PATCH RFC netfs] Fix uninitialized variable in netfs_retry_read_subrequests()

[Paul E. McKenney]

On Fri, Dec 20, 2024 at 12:20:11AM +0000, David Howells wrote:
> Paul E. McKenney <paulmck@kernel.org> wrote:
> 
> > This should actually be considered more of a bug report than a patch.
> > 
> > Clang 18.1.8 (but not GCC 11.5.0) complains that the "subreq" local
> > variable can be used uninitialized in netfs_retry_read_subrequests(),
> > just after the abandon_after label.  This function is unusual in having
> > three instances of this local variable.  The third and last one is clearly
> > erroneous because there is a branch out of the enclosing do-while loop
> > to the end of this function, and it looks like the intent is that the
> > code at the end of this function be using the same value of the "subreq"
> > local variable as is used within that do-while loop.
> > 
> > Therefore, take the obvious (if potentially quite misguided) approach
> > of removing the third declaration of "subreq", instead simply setting
> > it to NULL.
> 
> I think you're looking at the old version of my netfs-writeback branch that's
> residing in Christian's vfs.netfs branch.  I've posted a new version of my
> branch[1] without this problem and am hoping for Christian to update the
> branch[2] so that Stephen can pull it into linux-next.

Me too, and thank you for looking into this!

							Thanx, Paul

> David
> 
> [1] https://lore.kernel.org/linux-fsdevel/20241216204124.3752367-1-dhowells@redhat.com/T/#t
> 
> [2] And hoping he'll remember to drop "[PATCH v5 26/32] Display waited-on page
> index after 1min of waiting" for me.  I forgot to remove that debugging patch.
>