This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
On 19/08/2016 13:06, Zack Weinberg wrote:
> On Fri, Aug 19, 2016 at 10:00 AM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> On 19/08/2016 10:00, Zack Weinberg wrote:
>>>
>>> OK, what should I be using instead?
>>
>> [... some options which I will look into ...]
>>
>> That's why I think simpler solution would be just to remove this inline
>> optimization.
>
> It's really, really not simpler, though, because of the fortify issue...
>
>>> The *other* reason why I want to keep the inline optimization is that it
>>> means I don't need to add __explicit_bzero_chk (which, because of
>>> ifuncs, means futzing with *every sysdeps definition of memset*!). So
>>> either way there's going to be a new exported symbol.
>>
>> I do not think this is really required, you can still add a default
>> debug/explicit_bzero_chk.c which will call explicit_bzero/memset.
>
> You missed the point; the point was that if we don't add
> __glibc_read_memory then we have to add __explicit_bzero_chk, so
> there's one new implementation-namespace exported symbol either way.
>
> Anyway, it would be nice if that worked without modifying every
> definition of memset, but it doesn't. The problem is not that
> __explicit_bzero_chk itself would need to be an ifunc (I agree that
> that would be an unnecessary optimization); the problem is that
> __explicit_bzero_chk would need to call __memset_chk from inside
> libc.so. There are currently no calls to __memset_chk inside libc.so,
> which means there is no PLT bypass glue for __memset_chk. And because
> _some_ architectures define __memset_chk as an ifunc, it's in sysdeps,
> and _every_ memset.S has to be audited and possibly touched. This is
> such a PITA that in
> https://sourceware.org/ml/libc-alpha/2015-11/msg00467.html I appear to
> have only done it for x86.
>
> With the inline optimization, the fortified explicit_bzero can live
> entirely within bits/string3.h and no modifications to __memset_chk
> are required.
Well, current memset_chk default implementation just does:
if (__glibc_unlikely (dstlen < len))
__chk_fail ();
So it would be feasible imho to just this add and call memset instead
of memset_chk.
The problem, as pointed out by Florian, is not the symbol being exported
itself, but the inline definition where all the 'inline' definition might
either not being supported by the target compiler or have non-expected
side effects (as for C++ with -O0).
The fortified version is mostly fine to use 'inline' definition because
it is already guarded by _FORTIFY_SOURCE, but I think the default
explicit_bzero guarded by __USE_MISC is not suffice.