* [PATCH] xfs_io: don't assign cvtnum() return to unsigned var @ 2009-12-02 18:19 Eric Sandeen 2009-12-02 18:26 ` [PATCH V2] " Eric Sandeen 2009-12-03 16:58 ` [PATCH] " Christoph Hellwig 0 siblings, 2 replies; 5+ messages in thread From: Eric Sandeen @ 2009-12-02 18:19 UTC (permalink / raw) To: xfs-oss cvtnum() returns -1LL for unparseable values, but if we assign to a signed var, we can't test it: xfs_io> mincore 0 xxx range (0:0) is beyond mapping (0:1048576) Use a temporary signed var so we can detect the error: xfs_io> mincore 0 xxx non-numeric length argument -- xxx and also test whether it may overflow a size_t. Signed-off-by: Eric Sandeen <sandeen@sandeen.net> --- diff --git a/io/mincore.c b/io/mincore.c index f863f84..d534540 100644 --- a/io/mincore.c +++ b/io/mincore.c @@ -30,7 +30,7 @@ mincore_f( int argc, char **argv) { - off64_t offset; + off64_t offset, llength; size_t length; size_t blocksize, sectsize; void *start; @@ -49,12 +49,17 @@ mincore_f( argv[1]); return 0; } - length = cvtnum(blocksize, sectsize, argv[2]); - if (length < 0) { + llength = cvtnum(blocksize, sectsize, argv[2]); + if (llength < 0) { printf(_("non-numeric length argument -- %s\n"), argv[2]); return 0; - } + } else if (llength > (size_t)llength) { + printf(_("length argument too large -- %lld\n"), + llength); + return 0; + } else + length = (size_t)llength; } else { return command_usage(&mincore_cmd); } _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH V2] xfs_io: don't assign cvtnum() return to unsigned var 2009-12-02 18:19 [PATCH] xfs_io: don't assign cvtnum() return to unsigned var Eric Sandeen @ 2009-12-02 18:26 ` Eric Sandeen 2009-12-02 19:06 ` [PATCH V3] " Eric Sandeen 2009-12-03 16:58 ` [PATCH] " Christoph Hellwig 1 sibling, 1 reply; 5+ messages in thread From: Eric Sandeen @ 2009-12-02 18:26 UTC (permalink / raw) To: xfs-oss (whoops meant to include 2 fixes in that) cvtnum() returns -1LL for unparseable values, but if we assign to a signed var, we can't test it: There are problems in mincore & madvise. xfs_io> mincore 0 xxx range (0:0) is beyond mapping (0:1048576) Use a temporary signed var so we can detect the error: xfs_io> mincore 0 xxx non-numeric length argument -- xxx and also test whether it may overflow a size_t. Signed-off-by: Eric Sandeen <sandeen@sandeen.net> --- diff --git a/io/mincore.c b/io/mincore.c index f863f84..d534540 100644 --- a/io/mincore.c +++ b/io/mincore.c @@ -30,7 +30,7 @@ mincore_f( int argc, char **argv) { - off64_t offset; + off64_t offset, llength; size_t length; size_t blocksize, sectsize; void *start; @@ -49,12 +49,17 @@ mincore_f( argv[1]); return 0; } - length = cvtnum(blocksize, sectsize, argv[2]); - if (length < 0) { + llength = cvtnum(blocksize, sectsize, argv[2]); + if (llength < 0) { printf(_("non-numeric length argument -- %s\n"), argv[2]); return 0; - } + } else if (llength > (size_t)llength) { + printf(_("length argument too large -- %lld\n"), + llength); + return 0; + } else + length = (size_t)llength; } else { return command_usage(&mincore_cmd); } diff --git a/io/madvise.c b/io/madvise.c index 694cd41..cd16a4c 100644 --- a/io/madvise.c +++ b/io/madvise.c @@ -52,7 +52,7 @@ madvise_f( int argc, char **argv) { - off64_t offset; + off64_t offset, llength; size_t length; void *start; int advise = MADV_NORMAL, c; @@ -89,12 +89,17 @@ madvise_f( return 0; } optind++; - length = cvtnum(blocksize, sectsize, argv[optind]); - if (length < 0) { + llength = cvtnum(blocksize, sectsize, argv[optind]); + if (llength < 0) { printf(_("non-numeric length argument -- %s\n"), argv[optind]); return 0; - } + } else if (llength > (size_t)llength) { + printf(_("length argument too large -- %lld\n"), + llength); + return 0; + } else + length = (size_t)llength; } else { return command_usage(&madvise_cmd); } _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH V3] xfs_io: don't assign cvtnum() return to unsigned var 2009-12-02 18:26 ` [PATCH V2] " Eric Sandeen @ 2009-12-02 19:06 ` Eric Sandeen 2009-12-03 16:59 ` Christoph Hellwig 0 siblings, 1 reply; 5+ messages in thread From: Eric Sandeen @ 2009-12-02 19:06 UTC (permalink / raw) To: xfs-oss (whoops meant to include 2 fixes in that) (argh and fadvise too, I think this is the last one, sorry!) cvtnum() returns -1LL for unparseable values, but if we assign to a signed var, we can't test it: There are problems in fadvise, mincore & madvise. xfs_io> mincore 0 xxx range (0:0) is beyond mapping (0:1048576) For mincore & madvise, se a temporary signed var so we can detect the error: xfs_io> mincore 0 xxx non-numeric length argument -- xxx and also test whether it may overflow a size_t for mincore & madvise. For fadvise, posix_fadvise64 wants an off_t anyway so just switch to that. Signed-off-by: Eric Sandeen <sandeen@sandeen.net> --- diff --git a/io/mincore.c b/io/mincore.c index f863f84..d534540 100644 --- a/io/mincore.c +++ b/io/mincore.c @@ -30,7 +30,7 @@ mincore_f( int argc, char **argv) { - off64_t offset; + off64_t offset, llength; size_t length; size_t blocksize, sectsize; void *start; @@ -49,12 +49,17 @@ mincore_f( argv[1]); return 0; } - length = cvtnum(blocksize, sectsize, argv[2]); - if (length < 0) { + llength = cvtnum(blocksize, sectsize, argv[2]); + if (llength < 0) { printf(_("non-numeric length argument -- %s\n"), argv[2]); return 0; - } + } else if (llength > (size_t)llength) { + printf(_("length argument too large -- %lld\n"), + llength); + return 0; + } else + length = (size_t)llength; } else { return command_usage(&mincore_cmd); } diff --git a/io/madvise.c b/io/madvise.c index 694cd41..cd16a4c 100644 --- a/io/madvise.c +++ b/io/madvise.c @@ -52,7 +52,7 @@ madvise_f( int argc, char **argv) { - off64_t offset; + off64_t offset, llength; size_t length; void *start; int advise = MADV_NORMAL, c; @@ -89,12 +89,17 @@ madvise_f( return 0; } optind++; - length = cvtnum(blocksize, sectsize, argv[optind]); - if (length < 0) { + llength = cvtnum(blocksize, sectsize, argv[optind]); + if (llength < 0) { printf(_("non-numeric length argument -- %s\n"), argv[optind]); return 0; - } + } else if (llength > (size_t)llength) { + printf(_("length argument too large -- %lld\n"), + llength); + return 0; + } else + length = (size_t)llength; } else { return command_usage(&madvise_cmd); } diff --git a/io/fadvise.c b/io/fadvise.c index b5ab652..5a76ebb 100644 --- a/io/fadvise.c +++ b/io/fadvise.c @@ -52,8 +52,7 @@ fadvise_f( int argc, char **argv) { - off64_t offset = 0; - size_t length = 0; + off64_t offset = 0, length = 0; int c, range = 0, advise = POSIX_FADV_NORMAL; while ((c = getopt(argc, argv, "dnrsw")) != EOF) { _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V3] xfs_io: don't assign cvtnum() return to unsigned var 2009-12-02 19:06 ` [PATCH V3] " Eric Sandeen @ 2009-12-03 16:59 ` Christoph Hellwig 0 siblings, 0 replies; 5+ messages in thread From: Christoph Hellwig @ 2009-12-03 16:59 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs-oss On Wed, Dec 02, 2009 at 01:06:51PM -0600, Eric Sandeen wrote: > (whoops meant to include 2 fixes in that) > (argh and fadvise too, I think this is the last one, sorry!) > > cvtnum() returns -1LL for unparseable values, but if we > assign to a signed var, we can't test it: > > There are problems in fadvise, mincore & madvise. > > xfs_io> mincore 0 xxx > range (0:0) is beyond mapping (0:1048576) > > For mincore & madvise, se a temporary signed var so we > can detect the error: > > xfs_io> mincore 0 xxx > non-numeric length argument -- xxx > > and also test whether it may overflow a size_t for > mincore & madvise. > > For fadvise, posix_fadvise64 wants an off_t anyway so just > switch to that. Looks good even with all three covered, 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] 5+ messages in thread
* Re: [PATCH] xfs_io: don't assign cvtnum() return to unsigned var 2009-12-02 18:19 [PATCH] xfs_io: don't assign cvtnum() return to unsigned var Eric Sandeen 2009-12-02 18:26 ` [PATCH V2] " Eric Sandeen @ 2009-12-03 16:58 ` Christoph Hellwig 1 sibling, 0 replies; 5+ messages in thread From: Christoph Hellwig @ 2009-12-03 16:58 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs-oss On Wed, Dec 02, 2009 at 12:19:54PM -0600, Eric Sandeen wrote: > cvtnum() returns -1LL for unparseable values, but if we > assign to a signed var, we can't test it: > > xfs_io> mincore 0 xxx > range (0:0) is beyond mapping (0:1048576) > > Use a temporary signed var so we can detect the error: > > xfs_io> mincore 0 xxx > non-numeric length argument -- xxx > > and also test whether it may overflow a size_t. 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] 5+ messages in thread
end of thread, other threads:[~2009-12-03 16:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-02 18:19 [PATCH] xfs_io: don't assign cvtnum() return to unsigned var Eric Sandeen 2009-12-02 18:26 ` [PATCH V2] " Eric Sandeen 2009-12-02 19:06 ` [PATCH V3] " Eric Sandeen 2009-12-03 16:59 ` Christoph Hellwig 2009-12-03 16:58 ` [PATCH] " Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox