Received-SPF: pass (google.com: domain of linux-kernel+bounces-169121-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: David Laight <David.Laight@ACULAB.COM>
To: 'Linus Torvalds' <torvalds@linux-foundation.org>
CC: "axboe@kernel.dk" <axboe@kernel.dk>, "brauner@kernel.org"
	<brauner@kernel.org>, "christian.koenig@amd.com" <christian.koenig@amd.com>,
	"dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
	"io-uring@vger.kernel.org" <io-uring@vger.kernel.org>, "jack@suse.cz"
	<jack@suse.cz>, "keescook@chromium.org" <keescook@chromium.org>,
	"laura@labbott.name" <laura@labbott.name>, "linaro-mm-sig@lists.linaro.org"
	<linaro-mm-sig@lists.linaro.org>, "linux-fsdevel@vger.kernel.org"
	<linux-fsdevel@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "linux-media@vger.kernel.org"
	<linux-media@vger.kernel.org>, "minhquangbui99@gmail.com"
	<minhquangbui99@gmail.com>, "sumit.semwal@linaro.org"
	<sumit.semwal@linaro.org>,
	"syzbot+045b454ab35fd82a35fb@syzkaller.appspotmail.com"
	<syzbot+045b454ab35fd82a35fb@syzkaller.appspotmail.com>,
	"syzkaller-bugs@googlegroups.com" <syzkaller-bugs@googlegroups.com>,
	"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>
Subject: RE: [PATCH v2] epoll: be better about file lifetimes
Thread-Topic: [PATCH v2] epoll: be better about file lifetimes
Thread-Index: AQHanxYFUxCiqg1u6ES4w5URbGMRFrGJB6+A
Date: Sun, 5 May 2024 20:01:32 +0000
Message-ID: <12120faf79614fc1b9df272394a71550@AcuMS.aculab.com>
References: <CAHk-=wgMzzfPwKc=8yBdXwSkxoZMZroTCiLZTYESYD3BC_7rhQ@mail.gmail.com>
 <20240505175556.1213266-2-torvalds@linux-foundation.org>
In-Reply-To: <20240505175556.1213266-2-torvalds@linux-foundation.org>
Accept-Language: en-GB, en-US
Precedence: bulk
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

From: Linus Torvalds
> Sent: 05 May 2024 18:56
>=20
> epoll can call out to vfs_poll() with a file pointer that may race with
> the last 'fput()'. That would make f_count go down to zero, and while
> the ep->mtx locking means that the resulting file pointer tear-down will
> be blocked until the poll returns, it means that f_count is already
> dead, and any use of it won't actually get a reference to the file any
> more: it's dead regardless.
>=20
> Make sure we have a valid ref on the file pointer before we call down to
> vfs_poll() from the epoll routines.

How much is the extra pair of atomics going to hurt performance?
IIRC a lot of work was done to (try to) make epoll lockless.

Perhaps the 'hook' into epoll (usually) from sys_close should be
done before any of the references are removed?
(Which is different from Q6/A6 in man epoll - but that seems to be
documenting a bug!)
Then the ->poll() callback can't happen (assuming it is properly
locked) after the ->release() one.

It seems better to add extra atomics to the close/final-fput path
rather than ever ->poll() call epoll makes.

I can get extra references to a driver by dup() open("/dev/fd/n")
and mmap() - but epoll is definitely fd based.
(Which may be why it has the fd number in its data.)

Is there another race between EPOLL_CTL_ADD and close() (from a
different thread)?

=09David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1=
PT, UK
Registration No: 1397386 (Wales)