[PATCH 0/2] xfsprogs: Fix some issues found by checker

[PATCH 0/2] xfsprogs: Fix some issues found by checker

[Pavel Reichl]

Both issues have been found by Red Hat's covscan checker.

Pavel Reichl (2):
  xfs_db: Fix uninicialized error variable
  xfs_io: Fix do not loop through uninitialized var

 db/hash.c   | 2 +-
 io/parent.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.45.1

[PATCH 1/2] xfs_db: Fix uninicialized error variable

[Pavel Reichl]

To silence redhat's covscan checker:

Error: UNINIT (CWE-457): [#def1] [important]
xfsprogs-6.4.0/db/hash.c:308:2: var_decl: Declaring variable "error" without initializer.
xfsprogs-6.4.0/db/hash.c:353:2: uninit_use: Using uninitialized value "error".

Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
 db/hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/db/hash.c b/db/hash.c
index 05a94f24..9b3fdea6 100644
--- a/db/hash.c
+++ b/db/hash.c
@@ -304,7 +304,7 @@ collide_xattrs(
 	struct dup_table	*tab = NULL;
 	xfs_dahash_t		old_hash;
 	unsigned long		i;
-	int			error;
+	int			error = 0;
 
 	old_hash = libxfs_da_hashname((uint8_t *)name, namelen);
 
-- 
2.45.1

[PATCH 2/2] xfs_io: Fix do not loop through uninitialized var

[Pavel Reichl]

Red Hat's covscan checker found the following issue:

xfsprogs-6.4.0/io/parent.c:115:2: var_decl: Declaring variable "count" without initializer.
xfsprogs-6.4.0/io/parent.c:134:2: uninit_use: Using uninitialized value "count".

Currently, jdm_parentpaths() returns EOPNOTSUPP and does not initialize
the count variable. The count variable is subsequently used in a for
loop, which leads to undefined behavior. Fix this by returning from the
check_parents() function immediately after checking the return value of
the jdm_parentpaths() function.

Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
 io/parent.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io/parent.c b/io/parent.c
index 8f63607f..93f40997 100644
--- a/io/parent.c
+++ b/io/parent.c
@@ -112,7 +112,7 @@ check_parents(parent_t *parentbuf, size_t *parentbuf_size,
 	     jdm_fshandle_t *fshandlep, struct xfs_bstat *statp)
 {
 	int error, i;
-	__u32 count;
+	__u32 count = 0;
 	parent_t *entryp;
 
 	do {
@@ -126,7 +126,7 @@ check_parents(parent_t *parentbuf, size_t *parentbuf_size,
 			       (unsigned long long) statp->bs_ino,
 				strerror(errno));
 			err_status++;
-			break;
+			return;
 		}
 	} while (error == ERANGE);
 
-- 
2.45.1

Re: [PATCH 2/2] xfs_io: Fix do not loop through uninitialized var

[Darrick J. Wong]

On Fri, May 31, 2024 at 12:38:19AM +0200, Pavel Reichl wrote:
> Red Hat's covscan checker found the following issue:
> 
> xfsprogs-6.4.0/io/parent.c:115:2: var_decl: Declaring variable "count" without initializer.
> xfsprogs-6.4.0/io/parent.c:134:2: uninit_use: Using uninitialized value "count".
> 
> Currently, jdm_parentpaths() returns EOPNOTSUPP and does not initialize
> the count variable. The count variable is subsequently used in a for
> loop, which leads to undefined behavior. Fix this by returning from the
> check_parents() function immediately after checking the return value of
> the jdm_parentpaths() function.
> 
> Signed-off-by: Pavel Reichl <preichl@redhat.com>

I'm waiting on Carlos to take the xfsprogs 6.9 stuff so that I can
resend the new parent pointer code[1] for 6.10 which blows away the last
of the old SGI pptr code.

--D

[1] https://lore.kernel.org/linux-xfs/170405006341.1804688.11009892277015794783.stgit@frogsfrogsfrogs/

> ---
>  io/parent.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/io/parent.c b/io/parent.c
> index 8f63607f..93f40997 100644
> --- a/io/parent.c
> +++ b/io/parent.c
> @@ -112,7 +112,7 @@ check_parents(parent_t *parentbuf, size_t *parentbuf_size,
>  	     jdm_fshandle_t *fshandlep, struct xfs_bstat *statp)
>  {
>  	int error, i;
> -	__u32 count;
> +	__u32 count = 0;
>  	parent_t *entryp;
>  
>  	do {
> @@ -126,7 +126,7 @@ check_parents(parent_t *parentbuf, size_t *parentbuf_size,
>  			       (unsigned long long) statp->bs_ino,
>  				strerror(errno));
>  			err_status++;
> -			break;
> +			return;
>  		}
>  	} while (error == ERANGE);
>  
> -- 
> 2.45.1
> 
> 

Re: [PATCH 1/2] xfs_db: Fix uninicialized error variable

[Darrick J. Wong]

On Fri, May 31, 2024 at 12:38:18AM +0200, Pavel Reichl wrote:
> To silence redhat's covscan checker:
> 
> Error: UNINIT (CWE-457): [#def1] [important]
> xfsprogs-6.4.0/db/hash.c:308:2: var_decl: Declaring variable "error" without initializer.
> xfsprogs-6.4.0/db/hash.c:353:2: uninit_use: Using uninitialized value "error".
> 
> Signed-off-by: Pavel Reichl <preichl@redhat.com>

Looks good,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  db/hash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/db/hash.c b/db/hash.c
> index 05a94f24..9b3fdea6 100644
> --- a/db/hash.c
> +++ b/db/hash.c
> @@ -304,7 +304,7 @@ collide_xattrs(
>  	struct dup_table	*tab = NULL;
>  	xfs_dahash_t		old_hash;
>  	unsigned long		i;
> -	int			error;
> +	int			error = 0;
>  
>  	old_hash = libxfs_da_hashname((uint8_t *)name, namelen);
>  
> -- 
> 2.45.1
> 
> 

Re: [PATCH 1/2] xfs_db: Fix uninicialized error variable

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] xfs_io: Fix do not loop through uninitialized var

[Carlos Maiolino]

On Thu, May 30, 2024 at 03:48:42PM GMT, Darrick J. Wong wrote:
> On Fri, May 31, 2024 at 12:38:19AM +0200, Pavel Reichl wrote:
> > Red Hat's covscan checker found the following issue:
> >
> > xfsprogs-6.4.0/io/parent.c:115:2: var_decl: Declaring variable "count" without initializer.
> > xfsprogs-6.4.0/io/parent.c:134:2: uninit_use: Using uninitialized value "count".
> >
> > Currently, jdm_parentpaths() returns EOPNOTSUPP and does not initialize
> > the count variable. The count variable is subsequently used in a for
> > loop, which leads to undefined behavior. Fix this by returning from the
> > check_parents() function immediately after checking the return value of
> > the jdm_parentpaths() function.
> >
> > Signed-off-by: Pavel Reichl <preichl@redhat.com>
> 
> I'm waiting on Carlos to take the xfsprogs 6.9 stuff so that I can
> resend the new parent pointer code[1] for 6.10 which blows away the last
> of the old SGI pptr code.

I'm working on it :) sorry the delay should be ready most late next week.

 
> 
> --D
> 
> [1] https://lore.kernel.org/linux-xfs/170405006341.1804688.11009892277015794783.stgit@frogsfrogsfrogs/
> 
> > ---
> >  io/parent.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/io/parent.c b/io/parent.c
> > index 8f63607f..93f40997 100644
> > --- a/io/parent.c
> > +++ b/io/parent.c
> > @@ -112,7 +112,7 @@ check_parents(parent_t *parentbuf, size_t *parentbuf_size,
> >  	     jdm_fshandle_t *fshandlep, struct xfs_bstat *statp)
> >  {
> >  	int error, i;
> > -	__u32 count;
> > +	__u32 count = 0;
> >  	parent_t *entryp;
> >
> >  	do {
> > @@ -126,7 +126,7 @@ check_parents(parent_t *parentbuf, size_t *parentbuf_size,
> >  			       (unsigned long long) statp->bs_ino,
> >  				strerror(errno));
> >  			err_status++;
> > -			break;
> > +			return;
> >  		}
> >  	} while (error == ERANGE);
> >
> > --
> > 2.45.1
> >
> >
>