GNU bug report logs - #14530
[PATCH] od: -wN, N>64K, avoid misbehavior on systems with 32-bit size_t

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 14530 in the body.
You can then email your comments to 14530 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: bug-coreutils <at> gnu.org
Cc: Rich Burridge <rich.burridge <at> oracle.com>
Subject: [PATCH] od: -wN, N>64K,
	avoid misbehavior on systems with 32-bit size_t
Date: Sun, 02 Jun 2013 04:39:24 +0200

Rich reported this bug privately.

Here's a proposed patch.
I could have fixed the underlying problem with
explicit casts to a wider type (including floating point types),
but this seemed cleanest, if perhaps a little too subtle.
I'd rather not write a comment in each of three places,
and didn't really want to factor out a 3-input function
just to compute each next_pad value.

Along the way, I noticed that an unreasonably large width
caused trouble, but I'm not inclined to worry about it:

  $ : | od -An -a -w$(echo 2^32-1|bc)
  od: memory exhausted

From 19a4227793820f74bd6ca503ded3fafe65425eff Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> fb.com>
Date: Sat, 1 Jun 2013 19:20:06 -0700
Subject: [PATCH] od: -wN, N>64K, avoid misbehavior on systems with 32-bit
 size_t

* src/od.c (PRINT_FIELDS): Declare "i" to be of type uintmax_t, so that
the numerator in the expression for "next_pad" does not overflow.
(print_named_ascii): Likewise.
(print_ascii): Likewise.
Bug introduced via commit v6.12-42-g20c0b87.
* tests/misc/od.pl: Exercise each of the three affected code paths.
* NEWS (Bug fixes): Mention it.
Reported by Rich Burridge.
---
 NEWS             | 4 ++++
 THANKS.in        | 1 +
 src/od.c         | 6 +++---
 tests/misc/od.pl | 7 +++++++
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 721e05b..cad5eee 100644
--- a/NEWS
+++ b/NEWS
@@ -16,10 +16,14 @@ GNU coreutils NEWS                                    -*- outline -*-
   mkdir, mkfifo, and mknod now work better when creating a file in a directory
   with a default ACL whose umask disagrees with the process's umask, on a
   system such as GNU/Linux where directory ACL umasks override process umasks.
   [bug introduced in coreutils-6.0]

+  od -wN with N larger than 64K on a system with 32-bit size_t would
+  print approximately 2*N bytes of extraneous padding.
+  [Bug introduced in coreutils-7.0]
+
   tail --retry -f now waits for the files specified to appear.  Before, tail
   would immediately exit when such a file is inaccessible during the initial
   open.
   [This bug was introduced when inotify support was added in coreutils-7.5]

diff --git a/THANKS.in b/THANKS.in
index 67b60b9..f399d77 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -519,10 +519,11 @@ Rainer Orth                         ro <at> TechFak.Uni-Bielefeld.DE
 Ralf W. Stephan                     stephan <at> tmt.de
 Ralph Loader                        loader <at> maths.ox.ac.uk
 Rasmus Borup Hansen                 rbh <at> intomics.com
 Raul Miller                         moth <at> magenta.com
 Raúl Núñez de Arenas Coronado       raul <at> pleyades.net
+Rich Burridge                       rich.burridge <at> oracle.com
 Richard A Downing                   richard.downing <at> bcs.org.uk
 Richard Braakman                    dark <at> xs4all.nl
 Richard Dawe                        rich <at> phekda.freeserve.co.uk
 Richard J. Rauenzahn                rrauenza <at> hairball.cup.hp.com
 Richard Neill                       rn214 <at> hermes.cam.ac.uk
diff --git a/src/od.c b/src/od.c
index e8cab46..1c23401 100644
--- a/src/od.c
+++ b/src/od.c
@@ -398,11 +398,11 @@ and so on for G, T, P, E, Z, Y.\n\
 static void                                                             \
 N (size_t fields, size_t blank, void const *block,                      \
    char const *FMT_STRING, int width, int pad)                          \
 {                                                                       \
   T const *p = block;                                                   \
-  size_t i;                                                             \
+  uintmax_t i;                                                             \
   int pad_remaining = pad;                                              \
   for (i = fields; blank < i; i--)                                      \
     {                                                                   \
       int next_pad = pad * (i - 1) / fields;                            \
       int adjusted_width = pad_remaining - next_pad + width;            \
@@ -454,11 +454,11 @@ static void
 print_named_ascii (size_t fields, size_t blank, void const *block,
                    const char *unused_fmt_string _GL_UNUSED,
                    int width, int pad)
 {
   unsigned char const *p = block;
-  size_t i;
+  uintmax_t i;
   int pad_remaining = pad;
   for (i = fields; blank < i; i--)
     {
       int next_pad = pad * (i - 1) / fields;
       int masked_c = *p++ & 0x7f;
@@ -485,11 +485,11 @@ static void
 print_ascii (size_t fields, size_t blank, void const *block,
              const char *unused_fmt_string _GL_UNUSED, int width,
              int pad)
 {
   unsigned char const *p = block;
-  size_t i;
+  uintmax_t i;
   int pad_remaining = pad;
   for (i = fields; blank < i; i--)
     {
       int next_pad = pad * (i - 1) / fields;
       unsigned char c = *p++;
diff --git a/tests/misc/od.pl b/tests/misc/od.pl
index 0649b1c..fb579aa 100755
--- a/tests/misc/od.pl
+++ b/tests/misc/od.pl
@@ -55,10 +55,17 @@ my @Tests =

      # Ensure that od -j doesn't fseek across a nonempty file in /proc,
      # even if the kernel reports that the file has stat.st_size = 0.
      ['j-proc', "-An -c -j $proc_file_byte_count $proc_file",
                                {IN=>{f2=>'e'}}, {OUT=>"   e\n"}],
+
+     # Ensure that a large width does not cause trouble.
+     # From coreutils-7.0 through coreutils-8.21, these would print
+     # approximately 128KiB of padding.
+     ['wide-a',   '-a -w65537 -An', {IN=>{g=>'x'}}, {OUT=>"   x\n"}],
+     ['wide-c',   '-c -w65537 -An', {IN=>{g=>'x'}}, {OUT=>"   x\n"}],
+     ['wide-x', '-tx1 -w65537 -An', {IN=>{g=>'B'}}, {OUT=>" 42\n"}],
     );

 my $save_temps = $ENV{DEBUG};
 my $verbose = $ENV{VERBOSE};

--
1.8.3.101.g727a46b

Message #8 received at 14530 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 14530 <at> debbugs.gnu.org, Rich Burridge <rich.burridge <at> oracle.com>
Subject: Re: bug#14530: [PATCH] od: -wN, N>64K, avoid misbehavior on systems
	with 32-bit size_t
Date: Sun, 02 Jun 2013 16:16:04 +0100

On 06/02/2013 03:39 AM, Jim Meyering wrote:
> Rich reported this bug privately.
> 
> Here's a proposed patch.
> I could have fixed the underlying problem with
> explicit casts to a wider type (including floating point types),
> but this seemed cleanest, if perhaps a little too subtle.
> I'd rather not write a comment in each of three places,
> and didn't really want to factor out a 3-input function
> just to compute each next_pad value.

Since this is just a local variable,
I think the proposed solution is fine.


> Along the way, I noticed that an unreasonably large width
> caused trouble, but I'm not inclined to worry about it:
> 
>   $ : | od -An -a -w$(echo 2^32-1|bc)
>   od: memory exhausted

Yes this is an artificial limit, but in such
and edge case as I don't think it's worth
complicating the code for.

+1

thanks,
Pádraig.

Message #11 received at 14530 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 14530 <at> debbugs.gnu.org, Rich Burridge <rich.burridge <at> oracle.com>
Subject: Re: bug#14530: [PATCH] od: -wN, N>64K,
	avoid misbehavior on systems with 32-bit size_t
Date: Sun, 02 Jun 2013 18:31:54 +0200

close 14530
thanks

Pádraig Brady wrote:

> On 06/02/2013 03:39 AM, Jim Meyering wrote:
>> Rich reported this bug privately.
>>
>> Here's a proposed patch.
>> I could have fixed the underlying problem with
>> explicit casts to a wider type (including floating point types),
>> but this seemed cleanest, if perhaps a little too subtle.
>> I'd rather not write a comment in each of three places,
>> and didn't really want to factor out a 3-input function
>> just to compute each next_pad value.
>
> Since this is just a local variable,
> I think the proposed solution is fine.
>
>
>> Along the way, I noticed that an unreasonably large width
>> caused trouble, but I'm not inclined to worry about it:
>>
>>   $ : | od -An -a -w$(echo 2^32-1|bc)
>>   od: memory exhausted
>
> Yes this is an artificial limit, but in such
> and edge case as I don't think it's worth
> complicating the code for.
>
> +1

Thanks for the review.
Pushed.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.