[PATCH] xfs: fix return of uninitialized value in variable error

[PATCH] xfs: fix return of uninitialized value in variable error

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

A previous commit removed a call to xfs_attr3_leaf_read that
assigned an error return code to variable error. We now have
a few early error return paths to label 'out' that return
error if error is set; however error now is uninitialized
so potentially garbage is being returned.  Fix this by setting
error to zero to restore the original behaviour where error
was zero at the label 'restart'.

Addresses-Coverity: ("Uninitialized scalar variable")
Fixes: 07120f1abdff ("xfs: Add xfs_has_attr and subroutines")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/xfs/libxfs/xfs_attr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 472b3039eabb..902e5f7e6642 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -928,6 +928,7 @@ xfs_attr_node_addname(
 	 * Search to see if name already exists, and get back a pointer
 	 * to where it should go.
 	 */
+	error = 0;
 	retval = xfs_attr_node_hasname(args, &state);
 	if (retval != -ENOATTR && retval != -EEXIST)
 		goto out;
-- 
2.30.2

Re: [PATCH] xfs: fix return of uninitialized value in variable error

[Brian Foster]

On Fri, Apr 09, 2021 at 03:18:34PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> A previous commit removed a call to xfs_attr3_leaf_read that
> assigned an error return code to variable error. We now have
> a few early error return paths to label 'out' that return
> error if error is set; however error now is uninitialized
> so potentially garbage is being returned.  Fix this by setting
> error to zero to restore the original behaviour where error
> was zero at the label 'restart'.
> 
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: 07120f1abdff ("xfs: Add xfs_has_attr and subroutines")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  fs/xfs/libxfs/xfs_attr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 472b3039eabb..902e5f7e6642 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -928,6 +928,7 @@ xfs_attr_node_addname(
>  	 * Search to see if name already exists, and get back a pointer
>  	 * to where it should go.
>  	 */
> +	error = 0;
>  	retval = xfs_attr_node_hasname(args, &state);
>  	if (retval != -ENOATTR && retval != -EEXIST)
>  		goto out;

I think it would be nicer to initialize at the top of the function as
opposed to try and "preserve" historical behavior, but that nit aside:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> -- 
> 2.30.2
> 

Re: [PATCH] xfs: fix return of uninitialized value in variable error

[Colin Ian King]

On 09/04/2021 15:28, Brian Foster wrote:
> On Fri, Apr 09, 2021 at 03:18:34PM +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> A previous commit removed a call to xfs_attr3_leaf_read that
>> assigned an error return code to variable error. We now have
>> a few early error return paths to label 'out' that return
>> error if error is set; however error now is uninitialized
>> so potentially garbage is being returned.  Fix this by setting
>> error to zero to restore the original behaviour where error
>> was zero at the label 'restart'.
>>
>> Addresses-Coverity: ("Uninitialized scalar variable")
>> Fixes: 07120f1abdff ("xfs: Add xfs_has_attr and subroutines")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  fs/xfs/libxfs/xfs_attr.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index 472b3039eabb..902e5f7e6642 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -928,6 +928,7 @@ xfs_attr_node_addname(
>>  	 * Search to see if name already exists, and get back a pointer
>>  	 * to where it should go.
>>  	 */
>> +	error = 0;
>>  	retval = xfs_attr_node_hasname(args, &state);
>>  	if (retval != -ENOATTR && retval != -EEXIST)
>>  		goto out;
> 
> I think it would be nicer to initialize at the top of the function as
> opposed to try and "preserve" historical behavior, but that nit aside:

I did think about that, but this fix does ensure it's zero'd for each
iteration rather than just the once, so it should catch any code changes
later on that may loop back to this point were error is non-zero.

> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
>> -- 
>> 2.30.2
>>
> 

Re: [PATCH] xfs: fix return of uninitialized value in variable error

[Darrick J. Wong]

On Fri, Apr 09, 2021 at 03:18:34PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> A previous commit removed a call to xfs_attr3_leaf_read that
> assigned an error return code to variable error. We now have
> a few early error return paths to label 'out' that return
> error if error is set; however error now is uninitialized
> so potentially garbage is being returned.  Fix this by setting
> error to zero to restore the original behaviour where error
> was zero at the label 'restart'.
> 
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: 07120f1abdff ("xfs: Add xfs_has_attr and subroutines")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Looks correct to me...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_attr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 472b3039eabb..902e5f7e6642 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -928,6 +928,7 @@ xfs_attr_node_addname(
>  	 * Search to see if name already exists, and get back a pointer
>  	 * to where it should go.
>  	 */
> +	error = 0;
>  	retval = xfs_attr_node_hasname(args, &state);
>  	if (retval != -ENOATTR && retval != -EEXIST)
>  		goto out;
> -- 
> 2.30.2
>