You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
86 lines
4.8 KiB
86 lines
4.8 KiB
commit 6e8044e910600f71f4802dba2d105007af8428c3 |
|
Author: Joseph Myers <joseph@codesourcery.com> |
|
Date: Mon Nov 8 19:11:51 2021 +0000 |
|
|
|
Fix memmove call in vfprintf-internal.c:group_number |
|
|
|
A recent GCC mainline change introduces errors of the form: |
|
|
|
vfprintf-internal.c: In function 'group_number': |
|
vfprintf-internal.c:2093:15: error: 'memmove' specified bound between 9223372036854775808 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=] |
|
2093 | memmove (w, s, (front_ptr -s) * sizeof (CHAR_T)); |
|
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
This is a genuine bug in the glibc code: s > front_ptr is always true |
|
at this point in the code, and the intent is clearly for the |
|
subtraction to be the other way round. The other arguments to the |
|
memmove call here also appear to be wrong; w and s point just *after* |
|
the destination and source for copying the rest of the number, so the |
|
size needs to be subtracted to get appropriate pointers for the |
|
copying. Adjust the memmove call to conform to the apparent intent of |
|
the code, so fixing the -Wstringop-overflow error. |
|
|
|
Now, if the original code were ever executed, a buffer overrun would |
|
result. However, I believe this code (introduced in commit |
|
edc1686af0c0fc2eb535f1d38cdf63c1a5a03675, "vfprintf: Reuse work_buffer |
|
in group_number", so in glibc 2.26) is unreachable in prior glibc |
|
releases (so there is no need for a bug in Bugzilla, no need to |
|
consider any backports unless someone wants to build older glibc |
|
releases with GCC 12 and no possibility of this buffer overrun |
|
resulting in a security issue). |
|
|
|
work_buffer is 1000 bytes / 250 wide characters. This case is only |
|
reachable if an initial part of the number, plus a grouped copy of the |
|
rest of the number, fail to fit in that space; that is, if the grouped |
|
number fails to fit in the space. In the wide character case, |
|
grouping is always one wide character, so even with a locale (of which |
|
there aren't any in glibc) grouping every digit, a number would need |
|
to occupy at least 125 wide characters to overflow, and a 64-bit |
|
integer occupies at most 23 characters in octal including a leading 0. |
|
In the narrow character case, the multibyte encoding of the grouping |
|
separator would need to be at least 42 bytes to overflow, again |
|
supposing grouping every digit, but MB_LEN_MAX is 16. So even if we |
|
admit the case of artificially constructed locales not shipped with |
|
glibc, given that such a locale would need to use one of the character |
|
sets supported by glibc, this code cannot be reached at present. (And |
|
POSIX only actually specifies the ' flag for grouping for decimal |
|
output, though glibc acts on it for other bases as well.) |
|
|
|
With binary output (if you consider use of grouping there to be |
|
valid), you'd need a 15-byte multibyte character for overflow; I don't |
|
know if any supported character set has such a character (if, again, |
|
we admit constructed locales using grouping every digit and a grouping |
|
separator chosen to have a multibyte encoding as long as possible, as |
|
well as accepting use of grouping with binary), but given that we have |
|
this code at all (clearly it's not *correct*, or in accordance with |
|
the principle of avoiding arbitrary limits, to skip grouping on |
|
running out of internal space like that), I don't think it should need |
|
any further changes for binary printf support to go in. |
|
|
|
On the other hand, support for large sizes of _BitInt in printf (see |
|
the N2858 proposal) *would* require something to be done about such |
|
arbitrary limits (presumably using dynamic allocation in printf again, |
|
for sufficiently large _BitInt arguments only - currently only |
|
floating-point uses dynamic allocation, and, as previously discussed, |
|
that could actually be replaced by bounded allocation given smarter |
|
code). |
|
|
|
Tested with build-many-glibcs.py for aarch64-linux-gnu (GCC mainline). |
|
Also tested natively for x86_64. |
|
|
|
(cherry picked from commit db6c4935fae6005d46af413b32aa92f4f6059dce) |
|
|
|
diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c |
|
index 3f3d1e148a8e7fda..53d93b2f07ecb261 100644 |
|
--- a/stdio-common/vfprintf-internal.c |
|
+++ b/stdio-common/vfprintf-internal.c |
|
@@ -2154,7 +2154,8 @@ group_number (CHAR_T *front_ptr, CHAR_T *w, CHAR_T *rear_ptr, |
|
copy_rest: |
|
/* No further grouping to be done. Copy the rest of the |
|
number. */ |
|
- memmove (w, s, (front_ptr -s) * sizeof (CHAR_T)); |
|
+ w -= s - front_ptr; |
|
+ memmove (w, front_ptr, (s - front_ptr) * sizeof (CHAR_T)); |
|
break; |
|
} |
|
else if (*grouping != '\0')
|
|
|