[RFC PATCH 1/1] io: Add FORTIFY_SOURCE check for fcntl arguments

[Sergey Bugaev]

On Tue, May 23, 2023 at 10:57 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
> >>> +      if (__fcntl_cmd_needs_arg (__cmd) && __va_arg_pack_len () < 1)
> >>
> >> No implicit check for function that do not return bool:
> >>
> >>   if (__fcntl_cmd_needs_arg (__cmd) == 1 ...)
> >
> > Why? Is it just a code style thing?
>
> Yes, it is from the glibc code style (check 'Boolean Coercions' [1]).
>
> [1] https://sourceware.org/glibc/wiki/Style_and_Conventions

[I don't mean to argue, and surely you know better, and not that I
care about this], but I read that as saying that implicit
integer-to-bool conversions are frowned upon, not ints used as bools
(because C used to lack a bool type). In other words: if you have an
integer variable that can have many different values and you want to
check it against 0, you're supposed to write out the != 0 explicitly.
But if what you have is essentially a boolean, but you use int/1/0
instead of bool/true/false, then just checking if (my_bool) should be
fine, and doing != 0 would be awkward -- no?

I've surely seen glibc code use if (my_bool) without the != 0 when
my_bool is declared as an int. __libc_enable_secure is one example.

> > The idea here was that the 2-argument version of fcntl clearly does
> > not deal with time, either 32- or 64-bit...
>
> And that's why we don't have a __fcntl_time64 implementation.  But Florian
> has asked to add one anyway if we even eventually need to handle some time
> related structure.
>
> I think we can skip this for now, but at least add a comment stating that
> if we even need to handle time related field with fcntl we will a new
> redirection.

Hm, either I'm failing to understand what you're saying (which is
possible: it is late at night), or maybe I didn't make my point clear:

if what you're concerned is what the _2 functions end up calling
(__fcntl_time64 vs __libc_fcntl64), then it does not matter because
they're only used for 2-argument variant of fcntl, which can not
possibly deal with any time-related structures (nor any structures),
exactly because there's no third argument where a structure could be
passed. The 2-arg variant is basically for simple getters that return
an int.

But if something is off (wrt time64 vs not) in the main fortification
/ redirection code, then I surely need to fix that -- then please
clarify what specific configuration / case you're talking about.

Sergey