Re: [RFC PATCH v2 4/7] csu: Fix standard fds' mode

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 19/04/23 16:40, Sergey Bugaev wrote:
> On Wed, Apr 19, 2023, 22:13 Cristian Rodríguez <crrodriguez@opensuse.org> wrote:
>> Im a little bit lost on what it was supposed to do in this old form..  as the open flags are all wrong..
>>
>> Changelog says:
>>
>>  (__libc_check_standard_fds): Reverse modes so that common operations on
>>         the descriptors fail.
>>
>> So this was intended at some point in the past decades to make it fail..
> 
> Ah, I see, so I just failed to trace it through git blame / git log,
> because the commit renaming this file sysdeps/generic/check_fds.c ->
> csu/check_fds.c has been done wrong (or maybe this is an artefact of
> the migration to Git).
> 
> So which way would you prefer it, left as-is (i.e. without this patch)
> or switched back? If we leave it as is, we should at least add a
> comment explaining what's going on, for the next person who stumbles
> into this and also fails to trace it through git blame.
> 
>> I see it is used only for SUID statically linked binaries.
> 
> I might be missing something, but why statically linked only? I don't
> see anything like that in elf/Makefile (but maybe I don't know where
> to look, please tell me!), and also the same behavior is certainly
> exhibited by dynamically linked executables too. That ls -l I posted
> above is from a dynamic executable.

At least on Hurd, __libc_check_standard_fds is only built for !SHARED.

> 
>> is this really needed now? playing silly games with this fds will always result in silly prices.

My understanding of this code is to enforce that on setuid program with
stdin/stdout/stderr closed any operation fail.  The original commit
db33f7d4aef7 that added these had this specific comment:

  /* Protec SUID program against misuse of file descriptors.  */
  extern void __libc_check_standard_fds (void);

Maybe we can add extend the comment on this file to add the intention
of this code.