Re: llc: fix resource exhaustion in llc_conn_handler()

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
llc: fix resource exhaustion in llc_conn_handler()

This patch adds accept queue limit checking to llc_conn_handler() to
prevent resource exhaustion from unlimited socket creation. It adds
sk_acceptq_is_full() check before creating sockets and sk_acceptq_added()
after successful creation.

> diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
> index 5c0ac243b248..9296b5d6b04a 100644
> --- a/net/llc/llc_conn.c
> +++ b/net/llc/llc_conn.c
> @@ -802,10 +802,15 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
>  	 * in the newly created struct sock private area. -acme
>  	 */
>  	if (unlikely(sk->sk_state == TCP_LISTEN)) {
> -		struct sock *newsk = llc_create_incoming_sock(sk, skb->dev,
> -							      &saddr, &daddr);
> +		struct sock *newsk;
> +
> +		if (sk_acceptq_is_full(sk))
> +			goto drop_unlock;
> +		newsk = llc_create_incoming_sock(sk, skb->dev,
> +						 &saddr, &daddr);
>  		if (!newsk)
>  			goto drop_unlock;
> +		sk_acceptq_added(sk);
>  		skb_set_owner_r(skb, newsk);

After sk_acceptq_added(sk) increments the accept queue counter, this code
continues to llc_conn_rcv() or sk_add_backlog(). If sk_add_backlog() fails
later in the function (around line 833), the code goes to drop_unlock which
only frees the skb:

    drop_unlock:
        kfree_skb(skb);
        goto out;

Can newsk leak on this error path? The socket was created by
llc_create_incoming_sock() and added to the SAP via llc_sap_add_socket(),
but on sk_add_backlog() failure it appears never to be freed.

Additionally, sk_acceptq_added() incremented sk->sk_ack_backlog, but
drop_unlock does not call sk_acceptq_removed(). Does this cause the accept
queue counter to become permanently inflated?

For comparison, TCP handles this atomically in inet_csk_reqsk_queue_add()
where sk_acceptq_added() is called only after the socket is successfully
added to the accept queue, and inet_child_forget() cleans up on failure.
-- 
pw-bot: cr