V2 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]

[H.J. Lu]

On Mon, Mar 9, 2020 at 3:05 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu:
>
> > diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
> > index ca3b5849bc..2be7a3a49f 100644
> > --- a/sysdeps/x86/dl-cet.c
> > +++ b/sysdeps/x86/dl-cet.c
>
> > +           /* IBT is enabled only if it is enabled in executable as
> > +              well as all shared objects.  */
> > +           enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON
> > +                          || (l->l_cet & lc_ibt) != 0);
>
> As a general comment, I think we should add more logging as to way
> processes lose protection.  But that does not need to happen with this
> patch.
>
> > +               /* When IBT is enabled, we can't dlopening a shared
> > +                  object without IBT.  */
>
> “we cannot dlopen”

Fixed.

> > +               if (enable_ibt != ibt_enabled)
> > +                 _dl_signal_error (EINVAL, l->l_name, "dlopen",
> > +                                   N_("indirect branch tracking isn't enabled"));
>
> I think the error message should say *where* indirect branch tracking
> is not enabled.  I assume this is a property of the loaded object.

Fixed.   Now I got

.... libvirtd[1048]: internal error: Failed to load module
'/usr/lib64/libvirt/storage-backend/libvirt_storage_backend_rbd.so':
/usr/lib64/ceph/libceph-common.so.0: indirect branch tracking isn't
enabled: Invalid argument

>
> > +               /* When SHSTK is enabled, we can't dlopening a shared
> > +                  object without SHSTK.  */
> > +               if (enable_shstk != shstk_enabled)
> > +                 _dl_signal_error (EINVAL, l->l_name, "dlopen",
> > +                                   N_("shadow stack isn't enabled"));
>
> See above.

Also fixed.

> > diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c
> > new file mode 100644
> > index 0000000000..e6f5d6f145
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-7.c
>
> > +  funcp = mmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE,
> > +             MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > +  if (funcp == MAP_FAILED)
> > +    FAIL_EXIT1 ("mmap failed (errno=%d)", errno);
>
> You could use xmmap here.

Fixed,

> Rest of the patch looks okay to me.

Here is the updated patch.  OK for master?

Thanks.

--
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Remove-ARCH_CET_LEGACY_BITMAP-BZ-25397.patch
Type: text/x-patch
Size: 16437 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20200309/31e987bc/attachment-0001.bin>