netdev - Re: [PATCH net-next v3] net: ioctl: Use kernel memory on protocol ioctl callbacks

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: <a1074987-c3ce-56cd-3005-beb5a3c55ef9@kernel.org>
Date: Thu, 25 May 2023 09:34:12 -0600
From: David Ahern <dsahern@...nel.org>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
 Breno Leitao <leitao@...ian.org>
Cc: Remi Denis-Courmont <courmisch@...il.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Alexander Aring <alex.aring@...il.com>,
 Stefan Schmidt <stefan@...enfreihafen.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Matthieu Baerts <matthieu.baerts@...sares.net>,
 Mat Martineau <martineau@...nel.org>,
 Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
 Xin Long <lucien.xin@...il.com>, leit@...com, axboe@...nel.dk,
 asml.silence@...il.com, linux-kernel@...r.kernel.org,
 netdev@...r.kernel.org, dccp@...r.kernel.org, linux-wpan@...r.kernel.org,
 mptcp@...ts.linux.dev, linux-sctp@...r.kernel.org
Subject: Re: [PATCH net-next v3] net: ioctl: Use kernel memory on protocol
 ioctl callbacks

On 5/25/23 9:05 AM, Willem de Bruijn wrote:
>> +/* A wrapper around sock ioctls, which copies the data from userspace
>> + * (depending on the protocol/ioctl), and copies back the result to userspace.
>> + * The main motivation for this function is to pass kernel memory to the
>> + * protocol ioctl callbacks, instead of userspace memory.
>> + */
>> +int sk_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
>> +{
>> +       int rc = 1;
>> +
>> +       if (ipmr_is_sk(sk))
>> +               rc = ipmr_sk_ioctl(sk, cmd, arg);
>> +       else if (ip6mr_is_sk(sk))
>> +               rc = ip6mr_sk_ioctl(sk, cmd, arg);
>> +       else if (phonet_is_sk(sk))
>> +               rc = phonet_sk_ioctl(sk, cmd, arg);
> 
> I don't understand what this buys us vs testing the sk_family,
> sk_protocol and cmd here.

To keep protocol specific code out of core files is the reason I
suggested it.

> 
> It introduces even deeper dependencies on the protocol specific
> header files. And the CONFIG issues that result from that. And it
> adds a bunch of wrappers that are only used once.
>