bug#17455: [PATCH] shred: fix overflow checking of command-line options
Thanks for catching that! I used signed because I was worried about offbeat hosts where UINTMAX_MAX < OFF_T_MAX, which I think POSIX allows, but I suppose I was going overboard. There are still a few oddball hosts out there (I just the other day discovered that Unisys still ships POSIXish mainframes where UINTMAX_MAX == OFF_T_MAX, because off_t has 40 bits and uintmax_t has 39!) but nothing *that* weird as far as I know.
bug#17455: [PATCH] shred: fix overflow checking of command-line options
On 05/10/2014 08:39 PM, Jim Meyering wrote:
> On Sat, May 10, 2014 at 11:42 AM, Paul Eggert wrote:
>> > * src/shred.c (main): Limit -n (number of passes) value to
>> > ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long.
>> > Limit the -s (file size) value to OFF_T_MAX.
>> > ---
>> > src/shred.c | 9 +
>> > 1 file changed, 5 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/src/shred.c b/src/shred.c
>> > index 607c6be..f4347e0 100644
>> > --- a/src/shred.c
>> > +++ b/src/shred.c
> ...
>> > @@ -1256,9 +1256,10 @@ main (int argc, char **argv)
>> >
>> > case 's':
>> >{
>> > -uintmax_t tmp;
>> > -if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
>> > -!= LONGINT_OK)
>> > +intmax_t tmp;
>> > +if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
>> > + != LONGINT_OK)
>> > +|| OFF_T_MAX < tmp)
> Hi Paul,
> The above makes it so shred now accepts a negative size.
> Before, that would be diagnosed as invalid:
>
>$ shred -s-1 k
>shred: -1: invalid file size
>
> With a size of -2, shred will write 64KB blocks forever -- or until it
> runs out of space.
>
> Here's a patch to fix that and to add a test covering that case:
>
>
> 0001-shred-don-t-infloop-upon-negative-size.patch
>
>
> From d7cfcbef7eb2cd12ac83e5c1c123de2015adbef9 Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Sat, 10 May 2014 12:36:16 -0700
> Subject: [PATCH] shred: don't infloop upon negative size
>
> * src/shred.c (main): With the preceding change, shred -s-2 FILE
> would write 64KB blocks forever -- or until disk full. This change
> makes shred reject a negative size.
> * tests/misc/shred-negative.sh: New file.
> * tests/local.mk (all_tests): Add it.
> ---
> src/shred.c | 4 ++--
> tests/local.mk | 1 +
> tests/misc/shred-negative.sh | 28
> 3 files changed, 31 insertions(+), 2 deletions(-)
> create mode 100755 tests/misc/shred-negative.sh
>
> diff --git a/src/shred.c b/src/shred.c
> index f4347e0..bd88e38 100644
> --- a/src/shred.c
> +++ b/src/shred.c
> @@ -1256,8 +1256,8 @@ main (int argc, char **argv)
>
> case 's':
>{
> -intmax_t tmp;
> -if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
> +uintmax_t tmp;
> +if ((xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
> != LONGINT_OK)
> || OFF_T_MAX < tmp)
>{
> diff --git a/tests/local.mk b/tests/local.mk
> index 5286bfb..cd7da5b 100644
> --- a/tests/local.mk
> +++ b/tests/local.mk
> @@ -313,6 +313,7 @@ all_tests = \
>tests/misc/sha384sum.pl\
>tests/misc/sha512sum.pl\
>tests/misc/shred-exact.sh \
> + tests/misc/shred-negative.sh \
>tests/misc/shred-passes.sh \
>tests/misc/shred-remove.sh \
>tests/misc/shuf.sh \
> diff --git a/tests/misc/shred-negative.sh b/tests/misc/shred-negative.sh
> new file mode 100755
> index 000..86cbf3e
> --- /dev/null
> +++ b/tests/misc/shred-negative.sh
> @@ -0,0 +1,28 @@
> +#!/bin/sh
> +# Exercise shred -s-3 FILE
> +
> +# Copyright (C) 2014 Free Software Foundation, Inc.
> +
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see
bug#17455: [PATCH] shred: fix overflow checking of command-line options
On 05/10/2014 07:42 PM, Paul Eggert wrote:
> * src/shred.c (main): Limit -n (number of passes) value to
> ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long.
> Limit the -s (file size) value to OFF_T_MAX.
> ---
> src/shred.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/shred.c b/src/shred.c
> index 607c6be..f4347e0 100644
> --- a/src/shred.c
> +++ b/src/shred.c
> @@ -1231,7 +1231,7 @@ main (int argc, char **argv)
>{
> uintmax_t tmp;
> if (xstrtoumax (optarg, NULL, 10, &tmp, NULL) != LONGINT_OK
> -|| MIN (UINT32_MAX, SIZE_MAX / sizeof (int)) < tmp)
> +|| MIN (ULONG_MAX, SIZE_MAX / sizeof (int)) <= tmp)
Cool. I think the UNIT32 came from the first version
where ((word32)passes != passes) was used as a hacky
way to detect overflow.
>{
> error (EXIT_FAILURE, 0, _("%s: invalid number of passes"),
> quotearg_colon (optarg));
> @@ -1256,9 +1256,10 @@ main (int argc, char **argv)
>
> case 's':
>{
> -uintmax_t tmp;
> -if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
> -!= LONGINT_OK)
> +intmax_t tmp;
> +if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
> + != LONGINT_OK)
> +|| OFF_T_MAX < tmp)
>{
> error (EXIT_FAILURE, 0, _("%s: invalid file size"),
> quotearg_colon (optarg));
>
Ever since --size was added, it was stored and interpreted internally as off_t,
so the OFF_T_MAX guard is correct.
However the xstrtoumax() was used as a guard against negative numbers
which are now accepted :( I'll fixup with a revert to that bit.
thanks!
Pádraig.
bug#17455: [PATCH] shred: fix overflow checking of command-line options
On Sat, May 10, 2014 at 11:42 AM, Paul Eggert wrote:
> * src/shred.c (main): Limit -n (number of passes) value to
> ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long.
> Limit the -s (file size) value to OFF_T_MAX.
> ---
> src/shred.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/shred.c b/src/shred.c
> index 607c6be..f4347e0 100644
> --- a/src/shred.c
> +++ b/src/shred.c
...
> @@ -1256,9 +1256,10 @@ main (int argc, char **argv)
>
> case 's':
>{
> -uintmax_t tmp;
> -if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
> -!= LONGINT_OK)
> +intmax_t tmp;
> +if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
> + != LONGINT_OK)
> +|| OFF_T_MAX < tmp)
Hi Paul,
The above makes it so shred now accepts a negative size.
Before, that would be diagnosed as invalid:
$ shred -s-1 k
shred: -1: invalid file size
With a size of -2, shred will write 64KB blocks forever -- or until it
runs out of space.
Here's a patch to fix that and to add a test covering that case:
0001-shred-don-t-infloop-upon-negative-size.patch
Description: Binary data
bug#17451: Let's patch 'date' to suggest the FAQ for invalid date formats
This topic comes up often enough that I'm inclined to add an FAQ pointer
into that diagnostic issued by 'date'. Proposed patch is attached. I
suppose we could try for a more-stable URL, but one step at a time.
From 71f4e27b983b84589c06af234a071e1b36846699 Mon Sep 17 00:00:00 2001
From: Paul Eggert
Date: Sat, 10 May 2014 12:15:22 -0700
Subject: [PATCH] date: point people at the FAQ for invalid date formats
* src/date.c (invalid_date): New function.
(batch_convert, main): Use it.
---
src/date.c | 41 +
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/src/date.c b/src/date.c
index ef04cb5..d241228 100644
--- a/src/date.c
+++ b/src/date.c
@@ -260,6 +260,16 @@ Show the local time for 9AM next Friday on the west coast
of the US\n\
exit (status);
}
+static void
+invalid_date (char const *date, bool show_help)
+{
+ error (0, 0, _("invalid date %s"), quote (date));
+ if (show_help)
+error (0, 0, _("For help with date formats, please see:\n%s"),
+ ("http://www.gnu.org/software/coreutils/faq/coreutils-faq.html";
+"#The-date-command-is-not-working-right_002e"));
+}
+
/* Parse each line in INPUT_FILENAME as with --date and display each
resulting time and date. If the file cannot be opened, tell why
then exit. Issue a diagnostic for any lines that cannot be parsed.
@@ -268,11 +278,7 @@ Show the local time for 9AM next Friday on the west coast
of the US\n\
static bool
batch_convert (const char *input_filename, const char *format)
{
- bool ok;
FILE *in_stream;
- char *line;
- size_t buflen;
- struct timespec when;
if (STREQ (input_filename, "-"))
{
@@ -288,9 +294,10 @@ batch_convert (const char *input_filename, const char
*format)
}
}
- line = NULL;
- buflen = 0;
- ok = true;
+ char *line = NULL;
+ size_t buflen = 0;
+ bool ok = true;
+ bool show_help = true;
while (1)
{
ssize_t line_length = getline (&line, &buflen, in_stream);
@@ -300,16 +307,15 @@ batch_convert (const char *input_filename, const char
*format)
break;
}
- if (! parse_datetime (&when, line, NULL))
+ struct timespec when;
+ if (parse_datetime (&when, line, NULL))
+ok &= show_date (format, when);
+ else
{
if (line[line_length - 1] == '\n')
line[line_length - 1] = '\0';
- error (0, 0, _("invalid date %s"), quote (line));
- ok = false;
-}
- else
-{
- ok &= show_date (format, when);
+ invalid_date (line, show_help);
+ ok = show_help = false;
}
}
@@ -524,7 +530,10 @@ main (int argc, char **argv)
}
if (! valid_date)
-error (EXIT_FAILURE, 0, _("invalid date %s"), quote (datestr));
+{
+ invalid_date (datestr, true);
+ return EXIT_FAILURE;
+}
if (set_date)
{
@@ -540,7 +549,7 @@ main (int argc, char **argv)
ok &= show_date (format, when);
}
- exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
+ return ok ? EXIT_SUCCESS : EXIT_FAILURE;
}
/* Display the date and/or time in WHEN according to the format specified
--
1.9.0
bug#17451: date command: Bug?
tag 17451 notabug thanks On 05/09/2014 12:07 PM, Yin, Dazhong@ARB wrote: > When querying the time zone, the date command cannot deal with the hour when > time changes from standard time to daylight saving time. That's because that hour does not exist in your timezone. All of your examples look like correct behavior to me. You've asked a FAQ: https://www.gnu.org/software/coreutils/faq/coreutils-faq.html#The-date-command-is-not-working-right_002e Our advice is to compute dates in UTC, or when sticking to a locale with daylight savings, then compute dates relative to noon rather than near the hour that might disappear or be doubled. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
bug#17455: [PATCH] shred: fix overflow checking of command-line options
* src/shred.c (main): Limit -n (number of passes) value to
ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long.
Limit the -s (file size) value to OFF_T_MAX.
---
src/shred.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/shred.c b/src/shred.c
index 607c6be..f4347e0 100644
--- a/src/shred.c
+++ b/src/shred.c
@@ -1231,7 +1231,7 @@ main (int argc, char **argv)
{
uintmax_t tmp;
if (xstrtoumax (optarg, NULL, 10, &tmp, NULL) != LONGINT_OK
-|| MIN (UINT32_MAX, SIZE_MAX / sizeof (int)) < tmp)
+|| MIN (ULONG_MAX, SIZE_MAX / sizeof (int)) <= tmp)
{
error (EXIT_FAILURE, 0, _("%s: invalid number of passes"),
quotearg_colon (optarg));
@@ -1256,9 +1256,10 @@ main (int argc, char **argv)
case 's':
{
-uintmax_t tmp;
-if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
-!= LONGINT_OK)
+intmax_t tmp;
+if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0")
+ != LONGINT_OK)
+|| OFF_T_MAX < tmp)
{
error (EXIT_FAILURE, 0, _("%s: invalid file size"),
quotearg_colon (optarg));
--
1.9.0
bug#17450: multiplication bug
On 05/10/2014 06:50 PM, Bernhard Voelker wrote: > On 05/09/2014 05:51 PM, Pádraig Brady wrote: >> http://www.gnu.org/software/coreutils/faq/coreutils-faq.html#expr-2-_002a-3-does-not-work > > Hi Padraig, > > I don't know if this was discussed before: what do you think about > adding a sentence to the FAQ that some shells allow to turn off > globbing? E.g. in bash with 'set -f' or 'set -o noglob': > > $ ( set -o noglob && expr 2 * 2 ) > 4 Maybe, though you'd probably never do that in the context of calculations in a shell script, where escaping the '*' is the standard and portable way to achieve it. cheers, Pádraig.
bug#17450: multiplication bug
On 05/09/2014 05:51 PM, Pádraig Brady wrote: http://www.gnu.org/software/coreutils/faq/coreutils-faq.html#expr-2-_002a-3-does-not-work Hi Padraig, I don't know if this was discussed before: what do you think about adding a sentence to the FAQ that some shells allow to turn off globbing? E.g. in bash with 'set -f' or 'set -o noglob': $ ( set -o noglob && expr 2 * 2 ) 4 Have a nice day, Berny
bug#17451: date command: Bug?
When querying the time zone, the date command cannot deal with the hour when time changes from standard time to daylight saving time. Below examples show the tests done in the Pacific time zone. --- date -d "20120311 01" +%:::z -08 date -d "20120311 02" +%:::z date: invalid date `20120311 02' date -d "20120311 03" +%:::z -07 date -d "20140309 01" +%:::z -08 date -d "20140309 02" +%:::z date: invalid date `20140309 02' date -d "20140309 03" +%:::z -07
