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