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

[David Malcolm <dmalcolm@redhat.com>]

On Wed, 2022-07-13 at 16:01 +0200, Florian Weimer 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.

It turns out we don't warn for that.

GCC trunk's -fanalyzer implements the new warnings via a state machine
for file-descriptor values; it currently has rules for handling "open",
"close", "read", and "write", and these functions are currently hard-
coded inside the analyzer.

Here are some examples on Compiler Explorer of what it can/cannot
detect:
  https://godbolt.org/z/nqPadvM4f

Probably the most important one IMHO is the leak detection.

Would it be helpful to have some kind of attribute for "returns a new
open FD"?  Are there other ways to close a FD other than calling
"close" on it?  (Would converting that to some kind of "closes"
attribute be a good idea?)



> > 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 …

Are there any other "magic" values for file-descriptors we should be
aware of?

> 
> > -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.

Yeah, I don't think it's worth modeling things in that level of detail.
In particular, I don't want to get into modeling paths in the
filesystem, since that would be a huge scope-creep for the project.

Thanks
Dave

> 
> Thanks,
> Florian
>