Re: Adding file descriptor attribute(s) to gcc and glibc

[David Malcolm <dmalcolm@redhat.com>]

On Wed, 2022-07-13 at 22:26 +0530, Mir Immad wrote:
> Hi everyone,
> 
> Reading through the thread, I feel the following attributes look good
> and
> similar to what I've done:
> 
> __attribute__ ((fd_arg(N)))
> __attribute__ ((fd_arg_read(N)))
> __attribute__ ((fd_arg_write(N)))
> 
> I believe how to annotate the glibc headers is a separate discussion.
> 
> Dave:  From the GCC side of things, these three attributes are basic
> functionalities that can be useful for any piece of code that passes
> around
> file descriptors.

Immad: sounds good.  Please use the above names/syntax for the next
iteration of your patch to -fanalyzer.

Thanks!
Dave

> 
> Thanks
> Immad
> 
> 
> On Wed, Jul 13, 2022 at 7:31 PM Florian Weimer <fweimer@redhat.com>
> wrote:
> 
> > * David Malcolm:
> > 
> > > On Wed, 2022-07-13 at 14:05 +0200, Florian Weimer wrote:
> > > > * Szabolcs Nagy via Gcc:
> > > 
> > > [adding Immad back to the CC list]
> > > 
> > > > 
> > > > > to be honest, i'd expect interesting fd bugs to be
> > > > > dynamic and not easy to statically analyze.
> > > > > the use-after-unchecked-open maybe useful. i would
> > > > > not expect the access direction to catch many bugs.
> > > > 
> > > > You might be right.  But I think the annotations could help to
> > > > catch
> > > > use-after-close errors.
> > > > 
> > > > By the way, I think it would help us if we didn't have to
> > > > special-
> > > > case
> > > > AT_FDCWD using inline wrappers.
> > > 
> > > Florian: I confess I wasn't familiar with AT_FDCWD until I read
> > > your
> > > email and did a little reading a few minutes ago; it seems to be
> > > a
> > > "magic number" for an FD that has special meaning; on my system
> > > it has
> > > the value -100.
> > > 
> > > GCC's current implementation of the various -Wanalyzer-fd-*
> > > warnings
> > > will track state for constant integer values as well as symbolic
> > > values; it doesn't have any special meanings for specific integer
> > > values.  So e.g. it doesn't assume that 0, 1, and 2 have specific
> > > meaning or are opened with specific flags (the analysis doesn't
> > > necessarily begin its execution path at the start of "main", so
> > > there's
> > > no guarantee that the standard FDs have their standard meaning).
> > 
> > Ahh.  It might be useful to detect close (-1) etc. as a form of
> > double-close, and ther AT_FDCWD is exceptional.
> > 
> > > Presumably if someone attempts
> > >   close (AT_FDCWD);
> > > they'll get -1 and errno set to EBADFD, right?
> > 
> > Correct
> > 
> > > I don't think GCC's -fanalyzer needs to check for that.
> > 
> > Not sure …
> > 
> > > -fanalyzer's filedescriptor support doesn't yet have a concept of
> > > "directory filedescriptors".  Should it?  (similarly, it doesn't
> > > yet
> > > know about sockets)
> > > 
> > > A possible way to annotate "openat":
> > > 
> > >   int openat(int dirfd, const char *pathname, int flags)
> > >     __attr_fd_arg(1);
> > 
> > openat is not the most general interface in this regard.  We have
> > other
> > *at functions which accept an O_PATH descriptor (or maybe even a
> > different kind of non-directory descriptor) with pathname == "" and
> > AT_EMPTY_PATH.  I'm not sure if modeling all this is beneficial.
> > 
> > Thanks,
> > Florian
> > 
> >