Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <alpine.DEB.2.21.1911251322160.2408@hp-x360n> <CAHk-=wj_nbDN3piDJBEteUrjyrZCTA+CCk15NfV=5D2xFfGJGw@mail.gmail.com>
 <CAHk-=whn2Dp44tjUpLo9ogs=p-v-Vn7c2Xdo4p+2V=d1hTaiuA@mail.gmail.com>
In-Reply-To: <CAHk-=whn2Dp44tjUpLo9ogs=p-v-Vn7c2Xdo4p+2V=d1hTaiuA@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon, 25 Nov 2019 19:21:56 -0800
Message-ID: <CAHk-=wj3YSFT+C3n=7CTsB-8U0NUpTpT3xEH866H4-1qbQGw7Q@mail.gmail.com>
Subject: Re: Commit 0be0ee71 ("fs: properly and reliably lock f_pos in
 fdget_pos()") breaking userspace
To:     "Kenneth R. Crudup" <kenny@panix.com>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Kirill Smelkov <kirr@nexedi.com>
Content-Type: multipart/mixed; boundary="000000000000d582d60598376238"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

--000000000000d582d60598376238
Content-Type: text/plain; charset="UTF-8"

On Mon, Nov 25, 2019 at 5:58 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> It might still be a bit noisy even with the above, but I think it will
> at least be better.

Yeah, I think I see what's going on.

I for some reason entirely missed the tty case. Oops. That was just
stupid of me, I should have thought of it.

There are other things that trigger the new informational line in that
hacky patch, and they _might_ matter, but I suspect it's the tty and
sound cases that causes the worst problems.

I suspect Kirill Smelkov even might have mentioned the tty case at one
point, and I just spaced out.

There are other things too that trigger that debug check, like the
sound file descriptors, and they might well matter too.

Anyway, I think the thing to do (for now) is to just say "character
devices are FMODE_STREAM files if they have no llseek operations".
That should take care of both tty's and the sound devices.

You can certainly have a character device that can do llseek, but it
sounds like a reasonable base rule.

Of course, this may fix the f_pos locking issue, but replace it with a
"oops, the character device driver tried to look at *pos anyway", and
that will give you a nice OOPS instead.

So this patch might just replace the failure mode with another failure
mode instead. At which point I think I'd have to revert that "get rid
of FMODE_ATOMIC_POS" trial balloon, but let's see if this patch solves
your problem and is sufficient..

I'd suggest using it _together_ with that "pr_info()" debug patch I
sent, to see what else might be going on..

                Linus

--000000000000d582d60598376238
Content-Type: text/x-patch; charset="US-ASCII"; name="patch.diff"
Content-Disposition: attachment; filename="patch.diff"
Content-Transfer-Encoding: base64
Content-ID: <f_k3famq520>
X-Attachment-Id: f_k3famq520

IGZzL2NoYXJfZGV2LmMgfCAxMiArKysrKysrKysrKy0KIDEgZmlsZSBjaGFuZ2VkLCAxMSBpbnNl
cnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCgpkaWZmIC0tZ2l0IGEvZnMvY2hhcl9kZXYuYyBiL2Zz
L2NoYXJfZGV2LmMKaW5kZXggMDBkZmUxNzg3MWFjLi5lNWZmZWJkZjgwZDUgMTAwNjQ0Ci0tLSBh
L2ZzL2NoYXJfZGV2LmMKKysrIGIvZnMvY2hhcl9kZXYuYwpAQCAtMzY3LDYgKzM2NywxNiBAQCB2
b2lkIGNkZXZfcHV0KHN0cnVjdCBjZGV2ICpwKQogCX0KIH0KIAorc3RhdGljIGludCBtaWdodF9i
ZV9zdHJlYW0oc3RydWN0IGlub2RlICppbm9kZSwgc3RydWN0IGZpbGUgKmZpbGUpCit7CisJY29u
c3Qgc3RydWN0IGZpbGVfb3BlcmF0aW9ucyAqZm9wcyA9IGZpbGUtPmZfb3A7CisKKwlpZiAoZm9w
cy0+bGxzZWVrICYmIGZvcHMtPmxsc2VlayAhPSBub19sbHNlZWspCisJCXJldHVybiAwOworCisJ
cmV0dXJuIHN0cmVhbV9vcGVuKGlub2RlLCBmaWxlKTsKK30KKwogLyoKICAqIENhbGxlZCBldmVy
eSB0aW1lIGEgY2hhcmFjdGVyIHNwZWNpYWwgZmlsZSBpcyBvcGVuZWQKICAqLwpAQCAtNDE2LDcg
KzQyNiw3IEBAIHN0YXRpYyBpbnQgY2hyZGV2X29wZW4oc3RydWN0IGlub2RlICppbm9kZSwgc3Ry
dWN0IGZpbGUgKmZpbHApCiAJCQlnb3RvIG91dF9jZGV2X3B1dDsKIAl9CiAKLQlyZXR1cm4gMDsK
KwlyZXR1cm4gbWlnaHRfYmVfc3RyZWFtKGlub2RlLCBmaWxwKTsKIAogIG91dF9jZGV2X3B1dDoK
IAljZGV2X3B1dChwKTsK
--000000000000d582d60598376238--