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: <20191231150226.GA523748@light.dominikbrodowski.net>
 <20200101003017.GA116793@rani.riverdale.lan> <20200101183243.GB183871@rani.riverdale.lan>
In-Reply-To: <20200101183243.GB183871@rani.riverdale.lan>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 1 Jan 2020 13:39:00 -0800
Message-ID: <CAHk-=whzgLPi4szh8xOKysuS9CKaQESngc=n0omBVpwdQ822aw@mail.gmail.com>
Subject: Re: [PATCH] early init: open /dev/console with O_LARGEFILE
To:     Arvind Sankar <nivedita@alum.mit.edu>
Cc:     Dominik Brodowski <linux@dominikbrodowski.net>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        youling 257 <youling257@gmail.com>
Content-Type: multipart/mixed; boundary="0000000000008ed5e7059b1ae88f"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

--0000000000008ed5e7059b1ae88f
Content-Type: text/plain; charset="UTF-8"

On Wed, Jan 1, 2020 at 10:32 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> Also, this shouldn't impact the current issue I think, but won't doing
> filp_open followed by 3 f_dupfd's cause the file->f_count to become 4
> but with only 3 open fd's? Don't we have to do an fd_install for the
> first one and only 2 f_dupfd's?

I think *this* is the real reason for the odd regression.

Because we're leaking a file count, the original /dev/console stays
open, and we end up never calling file->f_op->release().

So we don't call tty_release() on that original /dev/console open, and
one effect of that is that we never then call session_clear_tty(),
which should make sure that all the processes in that session ID have
their controlling tty (signal->tty pointer) cleared.

And if that original controlling tty wasn't cleared, then subsequent
calls to set the controlling tty won't do anything, and who knows what
odd session leader confusion we might have.

youling, can you check if - instead of the revert - this simple 'add
an fput' fixes your warning.

I'm not saying that the revert is wrong at this point, but honestly,
I'd like to avoid the "we revert because we didn't understand what
went wrong". It would be good to go "Aaaahhhh, _that_ was the
problem".

                 Linus

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

IGluaXQvbWFpbi5jIHwgMiArKwogMSBmaWxlIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKQoKZGlm
ZiAtLWdpdCBhL2luaXQvbWFpbi5jIGIvaW5pdC9tYWluLmMKaW5kZXggMWVjZmQ0M2VkNDY0Li43
YjNhY2NiZDIyMTEgMTAwNjQ0Ci0tLSBhL2luaXQvbWFpbi5jCisrKyBiL2luaXQvbWFpbi5jCkBA
IC0xMTcxLDYgKzExNzEsOCBAQCB2b2lkIGNvbnNvbGVfb25fcm9vdGZzKHZvaWQpCiAJCWlmIChm
X2R1cGZkKGksIGZpbGUsIDApICE9IGkpCiAJCQlnb3RvIGVycl9vdXQ7CiAJfQorCS8qIHB1dCB0
aGUgb3JpZ2luYWwgcmVmIGZyb20gZmlscF9vcGVuLCBmX2R1cGZkKCkgaGFzIGluc3RhbGxlZCBu
ZXcgb25lcyAqLworCWZwdXQoZmlsZSk7CiAKIAlyZXR1cm47CiAK
--0000000000008ed5e7059b1ae88f--