* [PATCH v2 0/4] xfsprogs misc fixes
@ 2015-12-11 11:08 Vivek Trivedi
2015-12-11 11:09 ` [PATCH v2 1/4] xfsprogs: xfs_io: fix a memory leak in imap_f Vivek Trivedi
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Vivek Trivedi @ 2015-12-11 11:08 UTC (permalink / raw)
To: xfs; +Cc: Vivek Trivedi, a.sahrawat, pankaj.m
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/4] xfsprogs: xfs_io: fix a memory leak in imap_f
2015-12-11 11:08 [PATCH v2 0/4] xfsprogs misc fixes Vivek Trivedi
@ 2015-12-11 11:09 ` Vivek Trivedi
2015-12-20 15:33 ` Christoph Hellwig
2015-12-11 11:09 ` [PATCH v2 2/4] xfsprogs: fix integer overflow in xlog_find_verify_cycle Vivek Trivedi
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Vivek Trivedi @ 2015-12-11 11:09 UTC (permalink / raw)
To: xfs; +Cc: Vivek Trivedi, a.sahrawat, pankaj.m
add NULL check for malloc return and free allocated memory in
return path in imap_f
Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] xfsprogs: fix integer overflow in xlog_find_verify_cycle
2015-12-11 11:08 [PATCH v2 0/4] xfsprogs misc fixes Vivek Trivedi
2015-12-11 11:09 ` [PATCH v2 1/4] xfsprogs: xfs_io: fix a memory leak in imap_f Vivek Trivedi
@ 2015-12-11 11:09 ` Vivek Trivedi
2015-12-20 15:35 ` Christoph Hellwig
2015-12-11 11:09 ` [PATCH v2 3/4] xfsprogs: mkfs: fix unintentional integer overflow Vivek Trivedi
2015-12-11 11:09 ` [PATCH v2 4/4] xfsprogs: xfsdb: remove unnessary checks in process_leaf_node_dir_v2_free Vivek Trivedi
3 siblings, 1 reply; 12+ messages in thread
From: Vivek Trivedi @ 2015-12-11 11:09 UTC (permalink / raw)
To: xfs; +Cc: Vivek Trivedi, a.sahrawat, pankaj.m
Fix unintentional integer overflow in xlog_find_verify_cycle.
Reported by coverity.
Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] xfsprogs: mkfs: fix unintentional integer overflow
2015-12-11 11:08 [PATCH v2 0/4] xfsprogs misc fixes Vivek Trivedi
2015-12-11 11:09 ` [PATCH v2 1/4] xfsprogs: xfs_io: fix a memory leak in imap_f Vivek Trivedi
2015-12-11 11:09 ` [PATCH v2 2/4] xfsprogs: fix integer overflow in xlog_find_verify_cycle Vivek Trivedi
@ 2015-12-11 11:09 ` Vivek Trivedi
2015-12-20 15:36 ` Christoph Hellwig
2015-12-11 11:09 ` [PATCH v2 4/4] xfsprogs: xfsdb: remove unnessary checks in process_leaf_node_dir_v2_free Vivek Trivedi
3 siblings, 1 reply; 12+ messages in thread
From: Vivek Trivedi @ 2015-12-11 11:09 UTC (permalink / raw)
To: xfs; +Cc: Vivek Trivedi, a.sahrawat, pankaj.m
Fix unintentional integer overflow in mkfs.
reported by coverity.
Signed-off-by: Vivek Trivedi <t.vivek@samsung.com>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] xfsprogs: xfsdb: remove unnessary checks in process_leaf_node_dir_v2_free
2015-12-11 11:08 [PATCH v2 0/4] xfsprogs misc fixes Vivek Trivedi
` (2 preceding siblings ...)
2015-12-11 11:09 ` [PATCH v2 3/4] xfsprogs: mkfs: fix unintentional integer overflow Vivek Trivedi
@ 2015-12-11 11:09 ` Vivek Trivedi
2015-12-20 15:37 ` Christoph Hellwig
3 siblings, 1 reply; 12+ messages in thread
From: Vivek Trivedi @ 2015-12-11 11:09 UTC (permalink / raw)
To: xfs; +Cc: Vivek Trivedi, a.sahrawat, pankaj.m
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 <t.vivek@samsung.com>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] xfsprogs: xfs_io: fix a memory leak in imap_f
2015-12-11 11:09 ` [PATCH v2 1/4] xfsprogs: xfs_io: fix a memory leak in imap_f Vivek Trivedi
@ 2015-12-20 15:33 ` Christoph Hellwig
2015-12-20 23:48 ` Dave Chinner
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2015-12-20 15:33 UTC (permalink / raw)
To: Vivek Trivedi; +Cc: a.sahrawat, pankaj.m, xfs
> @@ -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
>
> 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.
Otherwise this looks good to me.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] xfsprogs: fix integer overflow in xlog_find_verify_cycle
2015-12-11 11:09 ` [PATCH v2 2/4] xfsprogs: fix integer overflow in xlog_find_verify_cycle Vivek Trivedi
@ 2015-12-20 15:35 ` Christoph Hellwig
2015-12-20 23:45 ` Dave Chinner
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2015-12-20 15:35 UTC (permalink / raw)
To: Vivek Trivedi; +Cc: a.sahrawat, pankaj.m, 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] xfsprogs: mkfs: fix unintentional integer overflow
2015-12-11 11:09 ` [PATCH v2 3/4] xfsprogs: mkfs: fix unintentional integer overflow Vivek Trivedi
@ 2015-12-20 15:36 ` Christoph Hellwig
2015-12-20 23:51 ` Dave Chinner
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2015-12-20 15:36 UTC (permalink / raw)
To: Vivek Trivedi; +Cc: a.sahrawat, pankaj.m, xfs
> +++ 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.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] xfsprogs: xfsdb: remove unnessary checks in process_leaf_node_dir_v2_free
2015-12-11 11:09 ` [PATCH v2 4/4] xfsprogs: xfsdb: remove unnessary checks in process_leaf_node_dir_v2_free Vivek Trivedi
@ 2015-12-20 15:37 ` Christoph Hellwig
0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-12-20 15:37 UTC (permalink / raw)
To: Vivek Trivedi; +Cc: a.sahrawat, pankaj.m, xfs
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] xfsprogs: fix integer overflow in xlog_find_verify_cycle
2015-12-20 15:35 ` Christoph Hellwig
@ 2015-12-20 23:45 ` Dave Chinner
0 siblings, 0 replies; 12+ messages in thread
From: Dave Chinner @ 2015-12-20 23:45 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: a.sahrawat, xfs, Vivek Trivedi, pankaj.m
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:
> > 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] xfsprogs: xfs_io: fix a memory leak in imap_f
2015-12-20 15:33 ` Christoph Hellwig
@ 2015-12-20 23:48 ` Dave Chinner
0 siblings, 0 replies; 12+ messages in thread
From: Dave Chinner @ 2015-12-20 23:48 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: a.sahrawat, xfs, Vivek Trivedi, pankaj.m
On Sun, Dec 20, 2015 at 07:33:57AM -0800, Christoph Hellwig wrote:
> > @@ -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
>
> >
> > 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] xfsprogs: mkfs: fix unintentional integer overflow
2015-12-20 15:36 ` Christoph Hellwig
@ 2015-12-20 23:51 ` Dave Chinner
0 siblings, 0 replies; 12+ messages in thread
From: Dave Chinner @ 2015-12-20 23:51 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: a.sahrawat, xfs, Vivek Trivedi, pankaj.m
On Sun, Dec 20, 2015 at 07:36:49AM -0800, Christoph Hellwig wrote:
> > +++ 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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-12-20 23:51 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-11 11:08 [PATCH v2 0/4] xfsprogs misc fixes Vivek Trivedi
2015-12-11 11:09 ` [PATCH v2 1/4] xfsprogs: xfs_io: fix a memory leak in imap_f Vivek Trivedi
2015-12-20 15:33 ` Christoph Hellwig
2015-12-20 23:48 ` Dave Chinner
2015-12-11 11:09 ` [PATCH v2 2/4] xfsprogs: fix integer overflow in xlog_find_verify_cycle Vivek Trivedi
2015-12-20 15:35 ` Christoph Hellwig
2015-12-20 23:45 ` Dave Chinner
2015-12-11 11:09 ` [PATCH v2 3/4] xfsprogs: mkfs: fix unintentional integer overflow Vivek Trivedi
2015-12-20 15:36 ` Christoph Hellwig
2015-12-20 23:51 ` Dave Chinner
2015-12-11 11:09 ` [PATCH v2 4/4] xfsprogs: xfsdb: remove unnessary checks in process_leaf_node_dir_v2_free Vivek Trivedi
2015-12-20 15:37 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.