netdev - Re: [PATCH net-next] net: devmem: drop iterator type check

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: <0665ead7-56b4-4066-a21e-9a759d9af38f@gmail.com>
Date: Mon, 19 May 2025 16:19:10 +0100
From: Pavel Begunkov <asml.silence@...il.com>
To: Stanislav Fomichev <stfomichev@...il.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, sagi@...mberg.me,
 willemb@...gle.com, almasrymina@...gle.com, kaiyuanz@...gle.com,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] net: devmem: drop iterator type check

On 5/19/25 15:41, Stanislav Fomichev wrote:
> On 05/19, Pavel Begunkov wrote:
>> On 5/16/25 23:54, Stanislav Fomichev wrote:
>>> sendmsg() with a single iov becomes ITER_UBUF, sendmsg() with multiple
>>> iovs becomes ITER_IOVEC. Instead of adjusting the check to include
>>> ITER_UBUF, drop the check completely. The callers are guaranteed
>>> to happen from system call side and we don't need to pay runtime
>>> cost to verify it.
>>
>> I asked for this because io_uring can pass bvecs. Only sendzc can
>> pass that with cmsg, so probably you won't be able to hit any
>> real issue, but io_uring needs and soon will have bvec support for
>> normal sends as well. One can argue we should care as it isn't
>> merged yet, but there is something very very wrong if an unrelated
>> and legal io_uring change is able to open a vulnerability in the
>> devmem path.
> 
> Any reason not to filter these out on the io_uring side? Or you'll
> have to interpret sendmsg flags again which is not nice?

Right, io_uring would need to walk cmsg for all sends, which is not
great for layering. And then it's really a devmem quirk that it uses
iterators in a non orthodox way, it'd be awkward to check a random
devmem restriction in io_uring, when otherwise they know nothing
about each other. And it's safer to keep local to devmem, because
try to remember if something changes, and what if there is someone
new passing non-iovec iter + cmsg in the future.

-- 
Pavel Begunkov