netdev - Re: [PATCH net] net: prevent address overwrite in connect() and sendmsg()

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: <1ca3ca8a-6185-bc55-de74-53991ffc6f91@iogearbox.net>
Date: Tue, 12 Sep 2023 16:22:07 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
 Jordan Rife <jrife@...gle.com>, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org
Cc: dborkman@...nel.org
Subject: Re: [PATCH net] net: prevent address overwrite in connect() and
 sendmsg()

On 9/12/23 3:33 PM, Willem de Bruijn wrote:
> Jordan Rife wrote:
>> commit 0bdf399342c5 ("net: Avoid address overwrite in kernel_connect")
>> ensured that kernel_connect() will not overwrite the address parameter
>> in cases where BPF connect hooks perform an address rewrite. However,
>> there remain other cases where BPF hooks can overwrite an address held
>> by a kernel client.
>>
>> ==Scenarios Tested==
>>
>> * Code in the SMB and Ceph modules calls sock->ops->connect() directly,
>>    allowing the address overwrite to occur. In the case of SMB, this can
>>    lead to broken mounts.
> 
> These should probably call kernel_connect instead.
> 
>> * NFS v3 mounts with proto=udp call sock_sendmsg() for each RPC call,
>>    passing a pointer to the mount address in msg->msg_name which is
>>    later overwritten by a BPF sendmsg hook. This can lead to broken NFS
>>    mounts.
> 
> Similarly, this could call kernel_sendmsg, and the extra copy handled
> in that wrapper. The arguments are not exacty the same, so not 100%
> this is feasible.
> 
> But it's preferable if in-kernel callers use the kernel_.. API rather
> than bypass it. Exactly for issues like the one you report.

Fully agree, if it's feasible it would be better to convert them over to
in-kernel API.

>> In order to more comprehensively fix this class of problems, this patch
>> pushes the address copy deeper into the stack and introduces an address
>> copy to both udp_sendmsg() and udpv6_sendmsg() to insulate all callers
>> from address rewrites.
>>
>> Signed-off-by: Jordan Rife <jrife@...gle.com>