Re: [PATCH] Linux: Add fortify wrapper for getdents64

[Florian Weimer <fweimer at redhat dot com>]

* Adhemerval Zanella:

> On 18/06/2019 11:38, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>>> +ssize_t
>>>> +__getdents64_chk (int fd, void *dst, size_t len, size_t dstlen)
>>>> +{
>>>> +  if (__glibc_unlikely (dstlen < len))
>>>> +    __chk_fail ();
>>>> +  return __getdents64 (fd, dst, len);
>>>> +}
>>>
>>> __chk_fail is exported in libc ABI, wouldn't be better to just inline it
>>> and avoid a new symbol?
>> 
>> None of the existing wrappers do this.  The advantage of not inling is
>> that you can look at a coredump and see __getdents64_chk in libc.so.6 in
>> the stack trace, even with (application) debugging symbols available.
>> 
>> Thanks,
>> Florian
>> 
>
> I am not sure why the __chk_fail was exported in first place, so I don't
> have a strong opinion whether we should use directly or not.  Either option
> should be fine.

It's hard to tell from the sources.  Historically, there are some
exports which should have been GLIBC_PRIVATE, but __chk_fail is probably
not one of those.

> As a side note, do you foresee this symbol being used extensively so
> we do need to add a fortify symbol? I noted some code tries to
> optimize directory listing by calling it directly instead of copying
> the buffer contents multiple times using readdir.

I think that we should add fortified versions for new functions where
this makes sense.  This function takes a variable-sized buffer, so I
think it qualifies purely on formal grounds.

The other question is whether we want to add fortification to all the
buffer-based NSS functions …

Thanks,
Florian