Bug 358111
| Summary: | glibc's printf(3) tries to allocate a 2^64-byte buffer | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Jim Meyering <meyering> | ||||||
| Component: | glibc | Assignee: | Jakub Jelinek <jakub> | ||||||
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
| Severity: | low | Docs Contact: | |||||||
| Priority: | low | ||||||||
| Version: | rawhide | CC: | drepper, fweimer | ||||||
| Target Milestone: | --- | ||||||||
| Target Release: | --- | ||||||||
| Hardware: | x86_64 | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2008-04-10 10:08:33 UTC | Type: | --- | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Attachments: |
|
||||||||
|
Description
Jim Meyering
2007-10-30 12:35:43 UTC
Created attachment 243321 [details]
add check of malloc return value
I looked at the code and see two problems.
First, an unchecked malloc, then there's the fact that
both prec and width are of type "int", so it looks like this expression can
overflow, when either approaches 2^31:
workstart = (CHAR_T *) malloc ((MAX (prec, width) + 32) * sizeof (CHAR_T));
besides, this is the only assignment of malloc'd memory to "workstart" that is
not checked. The patch below adds the missing check. This doesn't fix the bug,
but at least keeps glibc from trying to allocate so much memory a second time.
I've fixed the computation of the buffer size but I won't change the implementation to avoid the allocations since this only means that reasonable code doesn't run as well. We're not optimizing for the ridiculous. Hi Uli! Thanks for looking at that. However, don't you want the following patch, too? From my reading, your new __builtin_expect-0 test will always be false, since prec is a positive signed int value: thus guaranteed to be smaller than the unsigned SIZE_MAX-31. And even if an "int" value could approach the maximum size_t in magnitude, your "31" would then have to be "32". Otherwise, with prec = SIZE_MAX - 31, that test would fail (because it'd be ==, not > ) and the subsequent (size_t) prec + 32 would overflow to 0. attached patch below... Created attachment 295108 [details]
* stdio-common/vfprintf.c (vfprintf): Remove dead code.
No, that test is not dead, it's just incomplete. I've checked in a patch. This bug can be closed, I think. Thanks. That looks good now. For the record, even now, the test/block *is* still dead code in most cases: when sizeof CHAR_T == 1 || sizeof int < sizeof size_t. So it's *not* dead only for the wide-char functions and then, only when compiled with int and size_t having the same width. |