* [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] 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
* 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
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