linux-kernel - Re: [PATCH v1 2/2] epoll: Use __user_write_access_begin() and unsafe_put_user() in epoll_put

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 linux-cve-announce 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]

Message-ID: <ea7552f1-842c-4bb8-b19e-0410bf18c305@intel.com>
Date: Fri, 24 Oct 2025 07:05:50 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: alex@...ti.fr, aou@...s.berkeley.edu, axboe@...nel.dk, bp@...en8.de,
 brauner@...nel.org, catalin.marinas@....com, christophe.leroy@...roup.eu,
 dave.hansen@...ux.intel.com, edumazet@...gle.com, hpa@...or.com,
 kuni1840@...il.com, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
 linuxppc-dev@...ts.ozlabs.org, maddy@...ux.ibm.com, mingo@...hat.com,
 mpe@...erman.id.au, npiggin@...il.com, palmer@...belt.com, pjw@...nel.org,
 tglx@...utronix.de, torvalds@...ux-foundation.org, will@...nel.org,
 x86@...nel.org
Subject: Re: [PATCH v1 2/2] epoll: Use __user_write_access_begin() and
 unsafe_put_user() in epoll_put_uevent().

On 10/23/25 22:16, Kuniyuki Iwashima wrote:
>> This makes me nervous. The access_ok() check is quite a distance away.
>> I'd kinda want to see some performance numbers before doing this. Is
>> removing a single access_ok() even measurable?
> I noticed I made a typo in commit message, s/tcp_rr/udp_rr/.
> 
> epoll_put_uevent() can be called multiple times in a single
> epoll_wait(), and we can see 1.7% more pps on UDP even when
> 1 thread has 1000 sockets only:
> 
> server: $ udp_rr --nolog -6 -F 1000 -T 1 -l 3600
> client: $ udp_rr --nolog -6 -F 1000 -T 256 -l 3600 -c -H $SERVER
> server: $ nstat > /dev/null; sleep 10; nstat | grep -i udp
> 
> Without patch (2 stac/clac):
> Udp6InDatagrams                 2205209            0.0
> 
> With patch (1 stac/clac):
> Udp6InDatagrams                 2242602            0.0

I'm totally with you about removing a stac/clac:

	https://lore.kernel.org/lkml/20250228203722.CAEB63AC@davehans-spike.ostc.intel.com/

The thing I'm worried about is having the access_ok() so distant
from the unsafe_put_user(). I'm wondering if this:

-	__user_write_access_begin(uevent, sizeof(*uevent));
+	if (!user_write_access_begin(uevent, sizeof(*uevent))
+		return NULL;
	unsafe_put_user(revents, &uevent->events, efault);
	unsafe_put_user(data, &uevent->data, efault);
	user_access_end();

is measurably slower than what was in your series. If it is
not measurably slower, then the series gets simpler because it
does not need to refactor user_write_access_begin(). It also ends
up more obviously correct because the access check is closer to
the unsafe_put_user() calls.

Also, the extra access_ok() is *much* cheaper than stac/clac.