linux-kernel - Re: [PATCH net-next] xsk: Return -EINVAL instead of -EBUSY after xsk_get_pool_from

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]

Date:   Wed, 2 Jun 2021 10:29:58 +0200
From:   Magnus Karlsson <magnus.karlsson@...il.com>
To:     Wang Hai <wanghai38@...wei.com>
Cc:     Björn Töpel <bjorn@...nel.org>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] xsk: Return -EINVAL instead of -EBUSY after
 xsk_get_pool_from_qid() fails

On Wed, Jun 2, 2021 at 6:02 AM Wang Hai <wanghai38@...wei.com> wrote:
>
> xsk_get_pool_from_qid() fails not because the device's queues are busy,
> but because the queue_id exceeds the current number of queues.
> So when it fails, it is better to return -EINVAL instead of -EBUSY.
>
> Signed-off-by: Wang Hai <wanghai38@...wei.com>
> ---
>  net/xdp/xsk_buff_pool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 8de01aaac4a0..30ece117117a 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -135,7 +135,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
>                 return -EINVAL;
>
>         if (xsk_get_pool_from_qid(netdev, queue_id))
> -               return -EBUSY;
> +               return -EINVAL;

I guess your intent here is to return -EINVAL only when the queue_id
is larger than the number of active queues. But this patch also
changes the return code when the queue id is already in use and in
that case we should continue to return -EBUSY. As this function is
used by a number of drivers, the easiest way to accomplish this is to
introduce a test for queue_id out of bounds before this if-statement
and return -EINVAL there.

>         pool->netdev = netdev;
>         pool->queue_id = queue_id;
> --
> 2.17.1
>