[PATCH v2 0/4] xfsprogs misc fixes

minor fixes in xfsprogs, mostly coverity reported fixes.

Changes since v1:
 * to fix integer overflow warnings, change variable type instead
   of doing typecast

Vivek Trivedi (4):
  xfsprogs: xfs_io: fix a memory leak in imap_f
  xfsprogs: fix integer overflow in xlog_find_verify_cycle
  xfsprogs: mkfs: fix unintentional integer overflow
  xfsprogs: xfsdb: remove unnessary checks in
    process_leaf_node_dir_v2_free

 db/check.c                |    2 --
 io/imap.c                 |    7 ++++++-
 libxlog/xfs_log_recover.c |    2 +-
 mkfs/xfs_mkfs.c           |    2 +-
 4 files changed, 8 insertions(+), 5 deletions(-)

-- 
1.7.9.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

add NULL check for malloc return and free allocated memory in
return path in imap_f

Signed-off-by: Vivek Trivedi <redacted>
---
 io/imap.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/io/imap.c b/io/imap.c
index 34901cb..6ed98eb 100644
--- a/io/imap.c
+++ b/io/imap.c

@@ -39,6 +39,8 @@ imap_f(int argc, char **argv)
 		nent = atoi(argv[1]);
 
 	t = malloc(nent * sizeof(*t));
+	if (!t)
+		return 0;
 
 	bulkreq.lastip  = &last;
 	bulkreq.icount  = nent;

@@ -46,8 +48,10 @@ imap_f(int argc, char **argv)
 	bulkreq.ocount  = &count;
 
 	while (xfsctl(file->name, file->fd, XFS_IOC_FSINUMBERS, &bulkreq) == 0) {
-		if (count == 0)
+		if (count == 0) {
+			free(t);
 			return 0;
+		}
 		for (i = 0; i < count; i++) {
 			printf(_("ino %10llu count %2d mask %016llx\n"),
 				(unsigned long long)t[i].xi_startino,

@@ -55,6 +59,7 @@ imap_f(int argc, char **argv)
 				(unsigned long long)t[i].xi_allocmask);
 		}
 	}
+	free(t);
 	perror("xfsctl(XFS_IOC_FSINUMBERS)");
 	exitcode = 1;
 	return 0;

-- 
1.7.9.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Fix unintentional integer overflow in xlog_find_verify_cycle.
Reported by coverity.

Signed-off-by: Vivek Trivedi <redacted>
---
 libxlog/xfs_log_recover.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libxlog/xfs_log_recover.c b/libxlog/xfs_log_recover.c
index ef7cf68..6116ecd 100644
--- a/libxlog/xfs_log_recover.c
+++ b/libxlog/xfs_log_recover.c

@@ -244,7 +244,7 @@ xlog_find_verify_cycle(
 	xfs_daddr_t	i, j;
 	uint		cycle;
 	xfs_buf_t	*bp;
-	xfs_daddr_t	bufblks;
+	int		bufblks;
 	char		*buf = NULL;
 	int		error = 0;
 

-- 
1.7.9.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Fix unintentional integer overflow  in mkfs.
reported by coverity.

Signed-off-by: Vivek Trivedi <redacted>
---
 mkfs/xfs_mkfs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 7cba41a..35dafb4 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c

@@ -2022,7 +2022,7 @@ _("warning: sparse inodes not supported without CRC support, disabled.\n"));
 		 * and the underlying volume is striped, then set rtextblocks
 		 * to the stripe width.
 		 */
-		int		rswidth;
+		__uint64_t	rswidth;
 		__uint64_t	rtextbytes;
 
 		if (!norsflag && !xi.risfile && !(!rtsize && xi.disfile))

-- 
1.7.9.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

xfs_dir2_free_hdr_t entries are unsigned, so they can't be negative.
remove these unnessary checks in process_leaf_node_dir_v2_free.
Reported by coverity.

Signed-off-by: Vivek Trivedi <redacted>
---
 db/check.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/db/check.c b/db/check.c
index 9c1541d..9d986b9 100644
--- a/db/check.c
+++ b/db/check.c

@@ -3073,9 +3073,7 @@ process_leaf_node_dir_v3_free(
 		return;
 	}
 	if (be32_to_cpu(free->hdr.nvalid) > maxent || 
-				be32_to_cpu(free->hdr.nvalid) < 0 ||
 				be32_to_cpu(free->hdr.nused) > maxent || 
-				be32_to_cpu(free->hdr.nused) < 0 ||
 				be32_to_cpu(free->hdr.nused) > 
 					be32_to_cpu(free->hdr.nvalid)) {
 		if (!sflag || v)

-- 
1.7.9.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

please use a goto out_free; here

and place the free just before the return here so that we have a single
uwinding exit.

Otherwise this looks good to me.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

On Fri, Dec 11, 2015 at 04:39:01PM +0530, Vivek Trivedi wrote:

Fix unintentional integer overflow in xlog_find_verify_cycle.
Reported by coverity.

xfs_daddr_t is a 64-bit integer.  How does replacing it with an int fix
overlows?

xlog_find_verify_cycle originates in the kernel code, so any fix
should probably be done there first.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

This looks odd.  We initiallize assigned ft.rtswidth (which is an int)
or 0 to it.  I think you want a separate variable for the result of
the  DTOBT(rswidth) statement to make this clear.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Looks good,

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

On Sun, Dec 20, 2015 at 07:35:13AM -0800, Christoph Hellwig wrote:

On Fri, Dec 11, 2015 at 04:39:01PM +0530, Vivek Trivedi wrote:

quoted

Fix unintentional integer overflow in xlog_find_verify_cycle.
Reported by coverity.

xfs_daddr_t is a 64-bit integer.  How does replacing it with an int fix
overlows?

xfs_daddr_t is being abused here as a count rather than a disk
address, whereas all the other counts in the function are ints and
so the value can't be more than 32 bits anyway....

xlog_find_verify_cycle originates in the kernel code, so any fix
should probably be done there first.

I'll pull all the libxfs changes made first in userspace back into
the kernel - no point making people jump throw hoops just to get a
fix into the libxfs code...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

On Sun, Dec 20, 2015 at 07:33:57AM -0800, Christoph Hellwig wrote:

quoted

@@ -39,6 +39,8 @@ imap_f(int argc, char **argv)
-		if (count == 0)
+		if (count == 0) {
+			free(t);
 			return 0;
+		}

please use a goto out_free; here

quoted

 		for (i = 0; i < count; i++) {
 			printf(_("ino %10llu count %2d mask %016llx\n"),
 				(unsigned long long)t[i].xi_startino,

@@ -55,6 +59,7 @@ imap_f(int argc, char **argv)
 				(unsigned long long)t[i].xi_allocmask);
 		}
 	}
+	free(t);
 	perror("xfsctl(XFS_IOC_FSINUMBERS)");
 	exitcode = 1;
 	return 0;

and place the free just before the return here so that we have a single
uwinding exit.

<sigh>

I'll just make the change locally, given I *was* only 15 minutes
away from pushing this into the public repository.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

On Sun, Dec 20, 2015 at 07:36:49AM -0800, Christoph Hellwig wrote:

quoted

+++ b/mkfs/xfs_mkfs.c

@@ -2022,7 +2022,7 @@ _("warning: sparse inodes not supported without CRC support, disabled.\n"));
 		 * and the underlying volume is striped, then set rtextblocks
 		 * to the stripe width.
 		 */
-		int		rswidth;
+		__uint64_t	rswidth;
 		__uint64_t	rtextbytes;

This looks odd.  We initiallize assigned ft.rtswidth (which is an int)
or 0 to it.  I think you want a separate variable for the result of
the  DTOBT(rswidth) statement to make this clear.

I don't see any point in doing that. It really doesn't matter that
the variable it is initialised from is only a 32 bit int, and having
yet another variable in mkfs is not going to make the code easier to
understand...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs