lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 4 Aug 2020 17:27:17 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Joel Becker <jlbec@...lplan.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] configfs updates for 5.9

On Mon, Aug 3, 2020 at 7:07 AM Christoph Hellwig <hch@...radead.org> wrote:
>
>  - propagate binary attribute errors (Lenny Szubowicz)

I pulled this, but I then unpulled again.

The commit is completely confused and wrong.

In particular, this part:

+       /* Only commit the data if no more shared refs to file */
+       if (file_count(file) > 1)
+               return 0;

is bogus and prone to races, meaning that if there are multiple
openers, *none* of them flush.

You can have two threads that close() at (roughly) the same time, both
call ->flush(), and both of those see "file_count()" being 2 and never
do anything.

They then both call "fput()" and are done, and that's when the file
count gets decremented.

The fact is, "flush()" is called for each close(), and you should
flush any pending data. Checking for some file_count() thing is bogus
and completely wrong.

So make up your mind: either you flush synchronously at *every*
close() (->flush) and you can get error notification.

Or you flush at the last close (->release), but then you absolutely
cannot get any error reporting, because the last put of the file may
be long after the last close (because other things than just
open/close can increase the refcount).

You can't try to do some middle ground, because it _will_ be buggy.
You have the above two options, not some racy third one.

             Linus

Powered by blists - more mailing lists