| Message ID | 8756cc1083eb4cd93d3766cd39b2f34b6623bbcb.1606319495.git.szabolcs.nagy@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831] | expand |
On 27/11/2020 10:19, Szabolcs Nagy via Libc-alpha wrote: > The _dl_open_check and _rtld_main_check hooks are not called on the > dependencies of a loaded module, so BTI protection was missed on > every module other than the main executable and directly dlopened > libraries. > > The fix just iterates over dependencies to enable BTI. > > Fixes bug 26926. LGTM, modulus the argument name change. I also think it would be better to add a testcase, for both DT_NEEDED and dlopen case. > --- > sysdeps/aarch64/dl-bti.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c > index 196e462520..8f4728adce 100644 > --- a/sysdeps/aarch64/dl-bti.c > +++ b/sysdeps/aarch64/dl-bti.c > @@ -51,11 +51,24 @@ enable_bti (struct link_map *map, const char *program) > return 0; > } > > -/* Enable BTI for L if required. */ > +/* Enable BTI for MAP and its dependencies. */ > > void > -_dl_bti_check (struct link_map *l, const char *program) > +_dl_bti_check (struct link_map *map, const char *program) I don't see much gain changing the argument name. > { > - if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti) > - enable_bti (l, program); > + if (!GLRO(dl_aarch64_cpu_features).bti) > + return; > + > + if (map->l_mach.bti) > + enable_bti (map, program); > + > + unsigned int i = map->l_searchlist.r_nlist; > + while (i-- > 0) > + { > + struct link_map *l = map->l_initfini[i]; > + if (l->l_init_called) > + continue; > + if (l->l_mach.bti) > + enable_bti (l, program); > + } > } > Ok.
The 12/10/2020 14:51, Adhemerval Zanella wrote: > On 27/11/2020 10:19, Szabolcs Nagy via Libc-alpha wrote: > > The _dl_open_check and _rtld_main_check hooks are not called on the > > dependencies of a loaded module, so BTI protection was missed on > > every module other than the main executable and directly dlopened > > libraries. > > > > The fix just iterates over dependencies to enable BTI. > > > > Fixes bug 26926. > > LGTM, modulus the argument name change. > > I also think it would be better to add a testcase, for both DT_NEEDED > and dlopen case. thanks, i committed this with fixed argument name as attached. i plan to do tests later once i understand the BTI semantics (right now it's not clear if in the presence of some W^X policy BTI may be silently ignored or not).
diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c index 196e462520..8f4728adce 100644 --- a/sysdeps/aarch64/dl-bti.c +++ b/sysdeps/aarch64/dl-bti.c @@ -51,11 +51,24 @@ enable_bti (struct link_map *map, const char *program) return 0; } -/* Enable BTI for L if required. */ +/* Enable BTI for MAP and its dependencies. */ void -_dl_bti_check (struct link_map *l, const char *program) +_dl_bti_check (struct link_map *map, const char *program) { - if (GLRO(dl_aarch64_cpu_features).bti && l->l_mach.bti) - enable_bti (l, program); + if (!GLRO(dl_aarch64_cpu_features).bti) + return; + + if (map->l_mach.bti) + enable_bti (map, program); + + unsigned int i = map->l_searchlist.r_nlist; + while (i-- > 0) + { + struct link_map *l = map->l_initfini[i]; + if (l->l_init_called) + continue; + if (l->l_mach.bti) + enable_bti (l, program); + } }