This is the mail archive of the
glibc-bugs@sourceware.org
mailing list for the glibc project.
[Bug libc/18023] extend_alloca is broken (questionable pointer comparison, horrible machine code)
- From: "cvs-commit at gcc dot gnu.org" <sourceware-bugzilla at sourceware dot org>
- To: glibc-bugs at sourceware dot org
- Date: Mon, 07 Mar 2016 05:43:31 +0000
- Subject: [Bug libc/18023] extend_alloca is broken (questionable pointer comparison, horrible machine code)
- Auto-submitted: auto-generated
- References: <bug-18023-131 at http dot sourceware dot org/bugzilla/>
https://sourceware.org/bugzilla/show_bug.cgi?id=18023
--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".
The branch, fw/extend_alloca has been created
at 683543bbb3e2c1b17554c4096d00c2980f39a802 (commit)
- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=683543bbb3e2c1b17554c4096d00c2980f39a802
commit 683543bbb3e2c1b17554c4096d00c2980f39a802
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 23:22:45 2015 +0100
Remove macros extend_alloca, extend_alloca_account [BZ #18023]
And also the helper macro stackinfo_alloca_round.
extend_alloca simply does not work on x86_64 and current i386 because
its peculiar stack alignment rules.
Here's an analysis of the _dl_fini situation (before the removal of
extend_alloca).
Dump of assembler code for function _dl_fini:
<+0>: push %rbp
<+1>: mov %rsp,%rbp
<+4>: push %r15
<+6>: push %r14
<+8>: push %r13
<+10>: push %r12
<+12>: push %rbx
<+13>: sub $0x38,%rsp
The function pushes 6 registers on the stack and allocates 0x38 bytes,
which means that %rsp is a multiple of 16 after function prologue.
The initial alloca allocation does not change %rsp alignment:
<+210>: shr $0x4,%rcx
<+214>: shl $0x4,%rcx
<+218>: sub %rcx,%rsp
%r15 is the address of the previous stack allocation, it is used below.
This is the extend_alloca reallocation branch:
<+734>: add $0xf,%rdx
<+738>: and $0xfffffffffffffff0,%rdx
<+742>: lea 0x1e(%rdx),%rcx
<+746>: shr $0x4,%rcx
<+750>: shl $0x4,%rcx
<+754>: sub %rcx,%rsp
<+757>: lea 0xf(%rsp),%rcx
<+762>: and $0xfffffffffffffff0,%rcx
<+766>: lea (%rcx,%rdx,1),%rsi
<+770>: cmp %rsi,%r15
<+773>: je 0x7f963940b673 <_dl_fini+787>
<+775>: mov %rdx,-0x58(%rbp)
<+787>: add %rdx,-0x58(%rbp)
(a) %rdx, the new requested size, is rounded up to a multiple of 16
(+734, %+738), and the result is stored in %rdx@738.
(b) %rdx@738 + 31 is rounded down to a multiple of 16, the result is
stored in rcx@750 (+742, +746, +750). So %rcx@750 == %rdx@738 + 16.
(c) %rcx@750 bytes are allocated on the stack (+754). %rsp is rounded
upwards to a multiple of 16, result is stored in %rcx@762 (+757, +762).
This does not change the value of %rsp because it already was a multiple
of 16.
(d) %rsi@766 == %rcx@762 + %rdx@738 is compared against %r15. But this
comparison is always false because we allocated 16 extra bytes on the
stack in (b), which were reserved for the alignment in (c), but in fact
unused. We are left with a gap in stack usage, and the comparison is
always false.
(@XXX refers to register values after executing the instruction at
offset +XXX.)
If the alignment gap was actually used because of different alignment
for %rsp, then the comparison failure would still occur because the gap
would not have been added after this reallocation, but before the
previous allocation.
As a result, extend_alloca is never able to merge allocations. It also
turns out that the interface is difficult to use, especially in
cojunction with alloca account (which is rarely optional).
[BZ #18023]
* include/alloca.h (stackinfo_alloca_round, extend_alloca,
extend_alloca_account): Remove.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=7b4c16db30304b83a5d1e913d1a8f7e90a8c398c
commit 7b4c16db30304b83a5d1e913d1a8f7e90a8c398c
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 19:49:50 2015 +0100
glob: Rewrite to use struct scratch_buffer instead of extend_alloca
[BZ #18023]
* posix/glob.c (glob): Use struct scratch_buffer instead of
extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=488063238ee5c87b66c6982b1b6d508e30e44386
commit 488063238ee5c87b66c6982b1b6d508e30e44386
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 19:48:31 2015 +0100
wordexp: Rewrite parse_tilde to use struct scratch_buffer
[BZ #18023]
* posix/wordexp.c (parse_tilde): Use struct scratch_buffer
instead of extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f414b3f5947f264cb5d114965f284cacb2fb10b5
commit f414b3f5947f264cb5d114965f284cacb2fb10b5
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 19:38:42 2015 +0100
getaddrinfo: Use struct scratch_buffer instead of extend_alloca
This results in slightly smaller buffers in some cases, but as the
buffer size is passed to the called functions (and they will request
an increased buffer size with an ERANGE error code), this does not
result in a functional difference.
[BZ #18023]
* sysdeps/posix/getaddrinfo.c (gaih_inet_serv, gethosts)
(gaih_inet): Use struct scratch_buffer instead of extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=6de00dd8a3a76d0b7586393451e65ad6c2721a71
commit 6de00dd8a3a76d0b7586393451e65ad6c2721a71
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 19:14:29 2015 +0100
getlogin_r (Linux variant): Switch to struct scratch_buffer
This corrects the alloca accounting as a side effect. It was not off
if extend_alloca failed to merge allocations.
[BZ #18023]
* sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
Use struct scratch_buffer instead of extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=11c2a8bad9ca5fe510b73c0204b3dcf703f14d5c
commit 11c2a8bad9ca5fe510b73c0204b3dcf703f14d5c
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 19:11:55 2015 +0100
gethostid (Linux variant): Switch to struct scratch_buffer
Previously, extend_alloca was used without alloca accounting,
which could have been problematic with large NSS results.
[BZ #18023]
* sysdeps/unix/sysv/linux/gethostid.c (gethostid): Use struct
scratch_buffer instead of extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=9b71d3b4df6dd4e49f7638d1d936c921c50fa3d9
commit 9b71d3b4df6dd4e49f7638d1d936c921c50fa3d9
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 19:09:00 2015 +0100
nss_files: Use struct scratch_buffer instead of extend_alloca
In both _nss_files_gethostbyname3_r and _nss_files_initgroups_dyn,
__libc_use_alloca was misused because it was not taken into account
that extend_alloca can fail to merge allocations.
[BZ #18023]
* nss/nss_files/files-hosts.c (_nss_files_gethostbyname3_r):
Use struct scratch_buffer instead of extend_alloca.
* nss/nss_files/files-initgroups.c (_nss_files_initgroups_dyn):
Likewise.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=1af14faef808f03276766e5ee6d9ee7dc9053fba
commit 1af14faef808f03276766e5ee6d9ee7dc9053fba
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 19:03:01 2015 +0100
getent: Switch to struct scratch_buffer in initgroups_keys
The retry loop is slightly different here because getgrouplist
provides size information, so scratch_buffer_set_array_size can be
used to grow the buffer in a more precise fashion.
[BZ #18023]
* nss/getent.c (initgroups_keys): Use struct scratch_buffer
instead of extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=140d8711d446f5193b04b56e714ddf8d0eddbf62
commit 140d8711d446f5193b04b56e714ddf8d0eddbf62
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 18:59:48 2015 +0100
nscd: Switch to struct scratch_buffer in adhstaiX
The pre-allocation of the three scratch buffers increased the initial
stack size somewhat, but if retries are needed, the previous version
used more stack space if extend_alloca could not merge allocations.
Lack of alloca accounting also means could be problematic with
extremely large NSS responses, too.
[BZ #18023]
* nscd/aicache.c (addhstaiX): Use struct scratch_buffer instead
of extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=8825d4709a686a870d313cc602d489ddd5354a08
commit 8825d4709a686a870d313cc602d489ddd5354a08
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 18:55:33 2015 +0100
nscd: Use struct scratch_buffer instead of extend_alloca in most caches
This replaces the ERANGE retry loops with loops which have heap
fallback. Heap allocation might actually be required for extremely
large NSS results.
[BZ #18023]
* nscd/grpcache.c (addgrbyX): Use struct scratch_buffer instead
of extend_alloca.
* nscd/hstcache.c (addhstbyX): Likewise.
* nscd/pwdcache.c (addpwbyX): Likewise.
* nscd/servicescache.c (addservbyX): Likewise.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=e38bff4db6f03d1fab732737f43a25160c3e4703
commit e38bff4db6f03d1fab732737f43a25160c3e4703
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 16:18:21 2015 +0100
_nss_nis_initgroups_dyn: Use struct scratch_buffer instead of extend_alloca
Also adjusts the internal function get_uid.
[BZ #18023]
* nis/nss_nis/nis-initgroups.c (get_uid, _nss_nis_initgroups_dyn):
Use struct scratch_buffer instead of extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c95cc759ecb21f812872934ac55518aef28cf46b
commit c95cc759ecb21f812872934ac55518aef28cf46b
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 16:03:01 2015 +0100
_dl_map_object_deps: Use struct scratch_buffer instead of extend_alloca
The function comment suggests that _dl_map_object_deps cannot use
malloc, but it already allocates the l_initfini array on the heap, so
the additional allocation should be acceptable.
[BZ #18023]
* elf/dl-deps.c (_dl_map_object_deps): Use struct
scratch_buffer instead of extend_alloca.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=9bed8b7fca7867d3027b66ce3985a75136aed013
commit 9bed8b7fca7867d3027b66ce3985a75136aed013
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 15:14:19 2015 +0100
getgrent_next_nss (compat-initgroups): Remove alloca fallback
If the caller-supplied buffer is not large enough, fall back directly
malloc.
The previous __libc_use_alloca check was incorrect because it did not
take into account that extend_alloca may fail to merge allocations, so
it would underestimate the stack space being used by roughly a factor
of two.
[BZ #18023]
* nis/nss_compat/compat-initgroups.c (getgrent_next_nss): Fall
back to malloc directly, without stack allocations.
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=dc79f9aa56933dc8b475209f9a4059965b50ea26
commit dc79f9aa56933dc8b475209f9a4059965b50ea26
Author: Florian Weimer <fweimer@redhat.com>
Date: Sun Mar 1 18:29:47 2015 +0100
nscd restart: Use malloc instead of extend_alloca
This introduces a separate function, read_cmdline, which reads the
contents of /proc/self/cmdline into a heap-allocated buffer.
[BZ #18023]
* nscd/connections.c (read_cmdline): New function.
(restart): Use it. Update comment.
-----------------------------------------------------------------------
--
You are receiving this mail because:
You are on the CC list for the bug.