Re: KCSAN: data-race in __alloc_file / __alloc_file

[Eric Dumazet]

On Mon, Nov 11, 2019 at 10:31 AM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>
> On Mon, Nov 11, 2019 at 10:05 AM Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Mon, Nov 11, 2019 at 9:52 AM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> > >
> > > Now I wonder what to do with the ~400 KCSAN reports sitting in
> > > pre-moderation queue.
> >
> > So regular KASAN reports are fairly easy to deal with: they report
> > actual bugs. They may be hard to hit, but generally there's no
> > question about something like a use-after-free or whatever.
> >
> > The problem with KCSAN is that it's not clear how many of the reports
> > have been actual real honest-to-goodness bugs that could cause
> > problems, and how many of them are "this isn't actually a bug, but an
> > annotation will shut up KCSAN".
> >
> > My gut feeling would be that it would be best to ignore the ones that
> > are "an annotation will shut up KCSAN", and look at the ones that are
> > real bugs.
> >
> > Is there a pattern to those real bugs? Is there perhaps a way to make
> > KCSAN notice _that_ pattern in particular, and suppress the ones that
> > are "we can shut these up with annotations that don't really change
> > the code"?
> >
> > I think it would be much better for the kernel - and much better for
> > KCSAN - if the problem reports KCSAN reports are real problems that
> > can actually be triggered as problems, and that it behaves much more
> > like KASAN in that respect.
> >
> > Yes, yes, then once the *real* problems have been handled, maybe we
> > can expand the search to be "stylistic issues" and "in theory, this
> > could cause problems with a compiler that did X" issues.
> >
> > But I think the "just annotate" thing makes people more likely to
> > dismiss KCSAN issues, and I don't think it's healthy.
> >
>
> Problem is that KASAN/KCSAN stops as soon as one issue is hit,
> regardless of it being a false positive or not.
>
> (Same happens with LOCKDEP seeing only one issue, then disabling itself)
>
> If we do not annotate the false positive, the real issues might be
> hidden for years.
>
> There is no pattern really, only a lot of noise (small ' bugs'  that
> have no real impact)

An interesting case is the race in ksys_write()

if (ppos) {
     pos = *ppos; // data-race
     ppos = &pos;
}
ret = vfs_write(f.file, buf, count, ppos);
if (ret >= 0 && ppos)
    f.file->f_pos = pos;  // data-race

BUG: KCSAN: data-race in ksys_write / ksys_write

write to 0xffff8880a9c29568 of 8 bytes by task 27477 on cpu 1:
 ksys_write+0x101/0x1b0 fs/read_write.c:613
 __do_sys_write fs/read_write.c:623 [inline]
 __se_sys_write fs/read_write.c:620 [inline]
 __x64_sys_write+0x4c/0x60 fs/read_write.c:620
 do_syscall_64+0xcc/0x370 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x44/0xa9