[PATCH] net: phonet: fix BUG_ON() in pn_socket_autobind()

[PATCH] net: phonet: fix BUG_ON() in pn_socket_autobind()

[Deepanshu Kartikey]

pn_socket_autobind() calls pn_socket_bind() and treats
-EINVAL as a signal that the socket was already bound,
then uses BUG_ON() to verify it:

    if (err != -EINVAL)
        return err;
    BUG_ON(!pn_port(pn_sk(sock->sk)->sobject));

However, pn_socket_bind() returns -EINVAL in multiple
cases:

  1. address length too short
  2. socket not in TCP_CLOSE state
  3. socket already bound  <- only intended case

When -EINVAL comes from cases 1 or 2, sobject is still
zero (never assigned), causing BUG_ON to fire and crash
the kernel.

Fix this by checking the bound state directly via
pn_port(sobject) BEFORE calling pn_socket_bind(),
eliminating the ambiguous -EINVAL interpretation
entirely.

Reported-by: syzbot+706f5eb79044e686c794@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=706f5eb79044e686c794
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 net/phonet/socket.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index c4af26357144..5a55e7d14e85 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -204,14 +204,14 @@ static int pn_socket_autobind(struct socket *sock)
 	struct sockaddr_pn sa;
 	int err;
 
+	if (pn_port(pn_sk(sock->sk)->sobject))
+		return 0; /* socket was already bound */
+
 	memset(&sa, 0, sizeof(sa));
 	sa.spn_family = AF_PHONET;
 	err = pn_socket_bind(sock, (struct sockaddr_unsized *)&sa,
 			     sizeof(struct sockaddr_pn));
-	if (err != -EINVAL)
-		return err;
-	BUG_ON(!pn_port(pn_sk(sock->sk)->sobject));
-	return 0; /* socket was already bound */
+	return err;
 }
 
 static int pn_socket_connect(struct socket *sock, struct sockaddr_unsized *addr,
-- 
2.43.0

Re: [PATCH] net: phonet: fix BUG_ON() in pn_socket_autobind()

[Rémi Denis-Courmont]

Hi,

Le keskiviikkona 22. huhtikuuta 2026, 5.15.33 Itä-Euroopan kesäaika Deepanshu 
Kartikey a écrit :
> pn_socket_autobind() calls pn_socket_bind() and treats
> -EINVAL as a signal that the socket was already bound,
> then uses BUG_ON() to verify it:
> 
>     if (err != -EINVAL)
>         return err;
>     BUG_ON(!pn_port(pn_sk(sock->sk)->sobject));
> 
> However, pn_socket_bind() returns -EINVAL in multiple
> cases:
> 
>   1. address length too short
>   2. socket not in TCP_CLOSE state
>   3. socket already bound  <- only intended case
> 
> When -EINVAL comes from cases 1 or 2, sobject is still
> zero (never assigned), causing BUG_ON to fire and crash
> the kernel.
> 
> Fix this by checking the bound state directly via
> pn_port(sobject) BEFORE calling pn_socket_bind(),
> eliminating the ambiguous -EINVAL interpretation
> entirely.
> 
> Reported-by: syzbot+706f5eb79044e686c794@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=706f5eb79044e686c794
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>  net/phonet/socket.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/phonet/socket.c b/net/phonet/socket.c
> index c4af26357144..5a55e7d14e85 100644
> --- a/net/phonet/socket.c
> +++ b/net/phonet/socket.c
> @@ -204,14 +204,14 @@ static int pn_socket_autobind(struct socket *sock)
>  	struct sockaddr_pn sa;
>  	int err;
> 
> +	if (pn_port(pn_sk(sock->sk)->sobject))
> +		return 0; /* socket was already bound */
> +

This was almost 20 years ago, but IIRC, we did not do it that way back then 
because it results in a data race on sobject if another task binds the socket 
in parallel.
 
>  	memset(&sa, 0, sizeof(sa));
>  	sa.spn_family = AF_PHONET;
>  	err = pn_socket_bind(sock, (struct sockaddr_unsized *)&sa,
>  			     sizeof(struct sockaddr_pn));
> -	if (err != -EINVAL)
> -		return err;
> -	BUG_ON(!pn_port(pn_sk(sock->sk)->sobject));
> -	return 0; /* socket was already bound */
> +	return err;
>  }
> 
>  static int pn_socket_connect(struct socket *sock, struct sockaddr_unsized
> *addr,


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