On Tue, 12 Nov 2024 at 16:23, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, 12 Nov 2024 at 16:12, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > Ugh... Actually, I would rather mask that on fcntl side (and possibly
> > moved FMODE_RANDOM/FMODE_NOREUSE over there as well).
> >
> > Would make for simpler rules for locking - ->f_mode would be never
> > changed past open, ->f_flags would have all changes under ->f_lock.
>
> Yeah, sounds sane.
>
> That said, just looking at which bits are used in f_flags is a major
> PITA. About half the definitions use octal, with the other half using
> hex. Lovely.
>
> So I'd rather not touch that mess until we have to.
Actually, maybe the locking and the octal/hex mess should be
considered a reason to clean this all up early rather than ignore it.
Looking at that locking code in fadvise() just for the f_mode use does
make me think this would be a really good cleanup.
I note that our fcntl code seems buggy as-is, because while it does
use f_lock for assignments (good), it clearly does *not* use them for
reading.
So it looks like you can actually read inconsistent values.
I get the feeling that f_flags would want WRITE_ONCE/READ_ONCE in
_addition_ to the f_lock use it has.
The f_mode thing with fadvise() smells like the same bug. Just because
the modifications are serialized wrt each other doesn't mean that
readers are then automatically ok.
Linus
