All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rémi Denis-Courmont" <remi@remlab.net>
To: 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>,
	Morduan Zang <zhangdandan@uniontech.com>
Cc: Simon Horman <horms@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzkaller-bugs@googlegroups.com,
	syzbot+706f5eb79044e686c794@syzkaller.appspotmail.com,
	Morduan Zang <zhangdandan@uniontech.com>,
	zhanjun <zhanjun@uniontech.com>
Subject: Re: [PATCH] net: phonet: do not BUG_ON() in pn_socket_autobind() on failed bind
Date: Wed, 22 Apr 2026 18:18:48 +0300	[thread overview]
Message-ID: <2466095.vKB9LnXJlr@basile.remlab.net> (raw)
In-Reply-To: <81A6570B633FF6FE+20260422013807.63087-1-zhangdandan@uniontech.com>

Hi,

Le keskiviikkona 22. huhtikuuta 2026, 4.38.07 Itä-Euroopan kesäaika Morduan 
Zang a écrit :
> syzbot reported a kernel BUG triggered from pn_socket_sendmsg() via
> pn_socket_autobind():
> 
>   kernel BUG at net/phonet/socket.c:213!
>   RIP: 0010:pn_socket_autobind net/phonet/socket.c:213 [inline]
>   RIP: 0010:pn_socket_sendmsg+0x240/0x250 net/phonet/socket.c:421
>   Call Trace:
>    sock_sendmsg_nosec+0x112/0x150 net/socket.c:797
>    __sock_sendmsg net/socket.c:812 [inline]
>    __sys_sendto+0x402/0x590 net/socket.c:2280
>    ...
> 
> pn_socket_autobind() calls pn_socket_bind() with port 0 and, on
> -EINVAL, assumes the socket was already bound and asserts that the
> port is non-zero:
> 
>   err = pn_socket_bind(sock, ..., sizeof(struct sockaddr_pn));
>   if (err != -EINVAL)
>           return err;
>   BUG_ON(!pn_port(pn_sk(sock->sk)->sobject));
>   return 0; /* socket was already bound */
> 
> However pn_socket_bind() also returns -EINVAL when sk->sk_state is not
> TCP_CLOSE, even when the socket has never been bound and pn_port() is
> still 0.  In that case the BUG_ON() fires and panics the kernel from a
> user-triggerable path.
> 
> Treat the "bind returned -EINVAL but pn_port() is still 0" case as a
> regular error and propagate -EINVAL to the caller instead of crashing.
> Existing callers already translate a non-zero return from
> pn_socket_autobind() into -ENOBUFS/-EAGAIN, so returning -EINVAL here
> only changes behaviour from panic to a normal errno.
> 
> Fixes: ba113a94b750 ("Phonet: common socket glue")
> Reported-by: syzbot+706f5eb79044e686c794@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=706f5eb79044e686c794
> Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
> Signed-off-by: zhanjun <zhanjun@uniontech.com>
> ---
>  net/phonet/socket.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/net/phonet/socket.c b/net/phonet/socket.c
> index 4423d483c630..de9108adfe1c 100644
> --- a/net/phonet/socket.c
> +++ b/net/phonet/socket.c
> @@ -210,7 +210,15 @@ static int pn_socket_autobind(struct socket *sock)
>  			     sizeof(struct sockaddr_pn));
>  	if (err != -EINVAL)
>  		return err;
> -	BUG_ON(!pn_port(pn_sk(sock->sk)->sobject));
> +	/*
> +	 * pn_socket_bind() can return -EINVAL both when the socket is
> +	 * already bound (pn_port() != 0) and when sk_state != TCP_CLOSE
> +	 * without a prior bind.  Only the former is an "already bound"
> +	 * success for autobind; otherwise propagate -EINVAL instead of
> +	 * crashing the kernel.
> +	 */
> +	if (!pn_port(pn_sk(sock->sk)->sobject))
> +		return -EINVAL;

This could be written as just if (err != -EINVAL || unlikely(...)) return err;

>  	return 0; /* socket was already bound */
>  }


-- 
德尼-库尔蒙‧雷米
https://www.remlab.net/




  parent reply	other threads:[~2026-04-22 23:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-22  1:38 [PATCH] net: phonet: do not BUG_ON() in pn_socket_autobind() on failed bind Morduan Zang
2026-04-22  2:21 ` Deepanshu Kartikey
2026-04-22 15:18 ` Rémi Denis-Courmont [this message]
2026-04-23  0:47   ` Morduan Zang
2026-04-23  1:05 ` [PATCH net v2] " Morduan Zang
2026-04-24  8:01   ` Rémi Denis-Courmont
2026-04-28  1:50   ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2466095.vKB9LnXJlr@basile.remlab.net \
    --to=remi@remlab.net \
    --cc=courmisch@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=syzbot+706f5eb79044e686c794@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=zhangdandan@uniontech.com \
    --cc=zhanjun@uniontech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.