Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a1074987-c3ce-56cd-3005-beb5a3c55ef9@kernel.org>
Date:   Thu, 25 May 2023 09:34:12 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.10.0
Subject: Re: [PATCH net-next v3] net: ioctl: Use kernel memory on protocol
 ioctl callbacks
Content-Language: en-US
To:     Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
        Breno Leitao <leitao@debian.org>
Cc:     Remi Denis-Courmont <courmisch@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Alexander Aring <alex.aring@gmail.com>,
        Stefan Schmidt <stefan@datenfreihafen.org>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Matthieu Baerts <matthieu.baerts@tessares.net>,
        Mat Martineau <martineau@kernel.org>,
        Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
        Xin Long <lucien.xin@gmail.com>, leit@fb.com, axboe@kernel.dk,
        asml.silence@gmail.com, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, dccp@vger.kernel.org,
        linux-wpan@vger.kernel.org, mptcp@lists.linux.dev,
        linux-sctp@vger.kernel.org
References: <20230525125503.400797-1-leitao@debian.org>
 <CAF=yD-LHQNkgPb-R==53-2auVxkP9r=xqrz2A8oe61vkoDdWjg@mail.gmail.com>
From:   David Ahern <dsahern@kernel.org>
In-Reply-To: <CAF=yD-LHQNkgPb-R==53-2auVxkP9r=xqrz2A8oe61vkoDdWjg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.
>