| Message ID | 20150417192032.70DE42C3B91@topped-with-meat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 2015-04-17 at 12:20 -0700, Roland McGrath wrote: > Can you try this change (on branch roland/dl-nns) with that compiler? > I suspect a compile-time constant preventing evaluation of the > expressions doing indexing will avoid the warning. If it doesn't, > then the right thing to do is to put that inside #if DL_NNS > 1. > > While I was there I noticed that it's not properly checking for wildly > bogus NSID values that would make that indexing bogus at runtime (in > the SHARED case), so I put that in too. > > > Thanks, > Roland > > > 2015-04-17 Roland McGrath <roland@hack.frob.com> > > * elf/dl-open.c (_dl_open): Use __glibc_unlikely in invalid namespace > check. Reject NSID < 0 and NSID >= dl_nns, and check for DL_NNS==1, > before using NSID as an index. This patch did fix the problem in dl-open.c but dl-close.c has the same issue and would need the same fix. Steve Ellcey sellcey@imgtec.com
> This patch did fix the problem in dl-open.c but dl-close.c has the same > issue and would need the same fix. Please show that error.
On Fri, 2015-04-17 at 12:58 -0700, Roland McGrath wrote: > > This patch did fix the problem in dl-open.c but dl-close.c has the same > > issue and would need the same fix. > > Please show that error. dl-close.c: In function '_dl_close_worker': dl-close.c:136:42: error: array subscript is outside array bounds [-Werror=array-bounds] struct link_namespaces *ns = &GL(dl_ns)[nsid]; ^ cc1: all warnings being treated as errors
On Fri, 2015-04-17 at 13:02 -0700, Steve Ellcey wrote: > On Fri, 2015-04-17 at 12:58 -0700, Roland McGrath wrote: > > > This patch did fix the problem in dl-open.c but dl-close.c has the same > > > issue and would need the same fix. > > > > Please show that error. > > dl-close.c: In function '_dl_close_worker': > dl-close.c:136:42: error: array subscript is outside array bounds > [-Werror=array-bounds] > struct link_namespaces *ns = &GL(dl_ns)[nsid]; > ^ > cc1: all warnings being treated as errors Weird, I assumed that the dl-close.c issue was the same as the dl-open.c problem. But it looks different. After cutting it down with delta I get the following small test case and error. I do not see how GCC can know that nsid is not 0. Steve Ellcey sellcey@imgtec.com extern void bad (const char *__assertion) __attribute__ ((__nothrow__ )) __attribute__ ((__noreturn__)); struct link_map { long int l_ns; }; extern struct link_namespaces { unsigned int _ns_nloaded; } _dl_ns[1]; void _dl_close_worker (struct link_map *map) { long int nsid = map->l_ns; struct link_namespaces *ns = &_dl_ns[nsid]; (nsid != 0) ? (void) (0) : bad ("nsid != 0"); --ns->_ns_nloaded; % inst*/bin/*-gcc -O2 -Wall -Werror x.c x.c: In function '_dl_close_worker': x.c:10:39: error: array subscript is outside array bounds [-Werror=array-bounds] struct link_namespaces *ns = &_dl_ns[nsid]; ^ x.c:10:39: error: array subscript is outside array bounds [-Werror=array-bounds] cc1: all warnings being treated as errors
diff --git a/elf/dl-open.c b/elf/dl-open.c index 0dbe07f..2d0e082 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -619,8 +619,14 @@ no more namespaces available for dlmopen()")); /* Never allow loading a DSO in a namespace which is empty. Such direct placements is only causing problems. Also don't allow loading into a namespace used for auditing. */ - else if (__builtin_expect (nsid != LM_ID_BASE && nsid != __LM_ID_CALLER, 0) - && (GL(dl_ns)[nsid]._ns_nloaded == 0 + else if (__glibc_unlikely (nsid != LM_ID_BASE && nsid != __LM_ID_CALLER) + && (__glibc_unlikely (nsid < 0 || nsid >= GL(dl_nns)) + /* This prevents the [NSID] index expressions from being + evaluated, so the compiler won't think that we are + accessing an invalid index here in the !SHARED case where + DL_NNS is 1 and so any NSID != 0 is invalid. */ + || DL_NNS == 1 + || GL(dl_ns)[nsid]._ns_nloaded == 0 || GL(dl_ns)[nsid]._ns_loaded->l_auditing)) _dl_signal_error (EINVAL, file, NULL, N_("invalid target namespace in dlmopen()"));