On Mon, Nov 11, 2019 at 11:00 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> > if (ppos) {
> >      pos = *ppos; // data-race
>
> That code uses "fdget_pos().
>
> Which does mutual exclusion _if_ the file is something we care about
> pos for, and if it has more than one process using it.

That said, the more I look at that code, the less I like it.

I have this feeling we really should get rid of FMODE_ATOMIC_POS
entirely, now that we have the much nicer FMODE_STREAM to indicate
that 'pos' really doesn't matter.

Also, the test for "file_count(file) > 1" really is wrong, in that it
means that we protect against other processes, but not other threads.

So maybe we really should do the attached thing. Adding Al and Kirill
to the cc for comments. Kirill did some fairly in-depth review of the
whole locking on f_pos, it might be good to get his comments.

Al? Note the change from

-               if (file_count(file) > 1) {
+               if ((v & FDPUT_FPUT) || file_count(file) > 1) {

in __fdget_pos(). It basically says that the threaded case also does
the pos locking.

NOTE! This is entirely untested. It might be totally broken. It passes
my "LooksSuperficiallyFine(tm)" test, but that's all I'm going to say
about the patch.

            Linus