On 10/2/19 7:50 AM, Roland Illig wrote:
The current code says:
next_line_no = line_no + page_incr;
if (next_line_no < line_no)
die (EXIT_FAILURE, 0, _("line number overflow"));
Since intmax_t is a regular integer type, overflow invokes undefined
behavior and must therefore be checked using other means.
Thanks for the bug report. I looked for similar problems involving
integer-overflow diagnostics in coreutils and installed the attached
patches. The second patch should fix the bug you mentioned.
>From 1316620e81daf91317560226b2b63cbbf548c09d Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 3 Oct 2019 12:35:44 -0700
Subject: [PATCH 1/4] cp: simplify integer overflow checking
* src/copy.c (sparse_copy): Use INT_ADD_WRAPV instead
of doing overflow checking by hand.
---
src/copy.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 65cf65895..cd6104c7a 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -335,9 +335,7 @@ sparse_copy (int src_fd, int dest_fd, char *buf, size_t buf_size,
}
else /* Coalesce writes/seeks. */
{
- if (psize <= OFF_T_MAX - csize)
- psize += csize;
- else
+ if (INT_ADD_WRAPV (psize, csize, &psize))
{
error (0, 0, _("overflow reading %s"), quoteaf (src_name));
return false;
--
2.21.0
>From 89af2b307b455b53869bc9cf79af0272f7d8a1a2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 3 Oct 2019 12:37:12 -0700
Subject: [PATCH 2/4] nl: fix integer-overflow bug
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Problem reported by Roland Illig (Bug#37585)
* src/nl.c (print_lineno): Don’t rely on undefined behavior when
checking for integer overflow.
---
src/nl.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/nl.c b/src/nl.c
index 43092b4fe..d85408c8c 100644
--- a/src/nl.c
+++ b/src/nl.c
@@ -275,14 +275,10 @@ build_type_arg (char const **typep,
static void
print_lineno (void)
{
- intmax_t next_line_no;
-
printf (lineno_format, lineno_width, line_no, separator_str);
- next_line_no = line_no + page_incr;
- if (next_line_no < line_no)
+ if (INT_ADD_WRAPV (line_no, page_incr, &line_no))
die (EXIT_FAILURE, 0, _("line number overflow"));
- line_no = next_line_no;
}
/* Switch to a header section. */
--
2.21.0
>From 72a348cc2d6160aa24bca93c23b1a17ffb5b1366 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 3 Oct 2019 12:38:15 -0700
Subject: [PATCH 3/4] numfmt: avoid unlikely integer overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* src/numfmt.c (parse_format_string): Report overflow if
pad < -LONG_MAX, since that can’t be negated.
---
src/numfmt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/numfmt.c b/src/numfmt.c
index 305a88603..c56641cfd 100644
--- a/src/numfmt.c
+++ b/src/numfmt.c
@@ -1081,7 +1081,7 @@ parse_format_string (char const *fmt)
errno = 0;
pad = strtol (fmt + i, &endptr, 10);
- if (errno == ERANGE)
+ if (errno == ERANGE || pad < -LONG_MAX)
die (EXIT_FAILURE, 0,
_("invalid format %s (width overflow)"), quote (fmt));
--
2.21.0
>From d267ba04a6b4ad43e5a1311885f8ad9685502a5e Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Thu, 3 Oct 2019 12:41:22 -0700
Subject: [PATCH 4/4] truncate: avoid integer-overflow assumptions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* src/truncate.c (do_ftruncate): Simplify overflow checking,
and don’t rely on theoretically-nonportable assumptions
like assuming that OFF_MAX < UINTMAX_MAX.
---
src/truncate.c | 49 +++++++++++++++++++------------------------------
1 file changed, 19 insertions(+), 30 deletions(-)
diff --git a/src/truncate.c b/src/truncate.c
index 4494ab51a..e7fb8543a 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -116,31 +116,29 @@ do_ftruncate (int fd, char const *fname, off_t ssize, off_t rsize,
}
if (block_mode)
{
- off_t const blksize = ST_BLKSIZE (sb);
- if (ssize < OFF_T_MIN / blksize || ssize > OFF_T_MAX / blksize)
+ ptrdiff_t blksize = ST_BLKSIZE (sb);
+ intmax_t ssize0 = ssize;
+ if (INT_MULTIPLY_WRAPV (ssize, blksize, &ssize))
{
error (0, 0,
_("overflow in %" PRIdMAX
- " * %" PRIdMAX " byte blocks for file %s"),
- (intmax_t) ssize, (intmax_t) blksize,
- quoteaf (fname));
+ " * %" PRIdPTR " byte blocks for file %s"),
+ ssize0, blksize, quoteaf (fname));
return false;
}
- ssize *= blksize;
}
if (rel_mode)
{
- uintmax_t fsize;
+ off_t fsize;
if (0 <= rsize)
fsize = rsize;
else
{
- off_t file_size;
if (usable_st_size (&sb))
{
- file_size = sb.st_size;
- if (file_size < 0)
+ fsize = sb.st_size;
+ if (fsize < 0)
{
/* Sanity check. Overflow is the only reason I can think
this would ever go negative. */
@@ -151,46 +149,37 @@ do_ftruncate (int fd, char const *fname, off_t ssize, off_t rsize,
}
else
{
- file_size = lseek (fd, 0, SEEK_END);
- if (file_size < 0)
+ fsize = lseek (fd, 0, SEEK_END);
+ if (fsize < 0)
{
error (0, errno, _("cannot get the size of %s"),
quoteaf (fname));
return false;
}
}
- fsize = file_size;
}
if (rel_mode == rm_min)
- nsize = MAX (fsize, (uintmax_t) ssize);
+ nsize = MAX (fsize, ssize);
else if (rel_mode == rm_max)
- nsize = MIN (fsize, (uintmax_t) ssize);
+ nsize = MIN (fsize, ssize);
else if (rel_mode == rm_rdn)
/* 0..ssize-1 -> 0 */
- nsize = (fsize / ssize) * ssize;
- else if (rel_mode == rm_rup)
- /* 1..ssize -> ssize */
+ nsize = fsize - fsize % ssize;
+ else
{
- /* Here ssize>=1 && fsize>=0 */
- uintmax_t const overflow = ((fsize + ssize - 1) / ssize) * ssize;
- if (overflow > OFF_T_MAX)
+ if (rel_mode == rm_rup)
{
- error (0, 0, _("overflow rounding up size of file %s"),
- quoteaf (fname));
- return false;
+ /* 1..ssize -> ssize */
+ off_t r = fsize % ssize;
+ ssize = r == 0 ? 0 : ssize - r;
}
- nsize = overflow;
- }
- else
- {
- if (ssize > OFF_T_MAX - (off_t)fsize)
+ if (INT_ADD_WRAPV (fsize, ssize, &nsize))
{
error (0, 0, _("overflow extending size of file %s"),
quoteaf (fname));
return false;
}
- nsize = fsize + ssize;
}
}
else
--
2.21.0
