Re: [PATCH] net: phonet: do not BUG_ON() in pn_socket_autobind() on failed bind

[Deepanshu Kartikey]

Hi Morduan,

Thanks for fixing this syzbot report!

I independently worked on the same bug and sent an
alternative patch here:

https://lore.kernel.org/all/20260422021533.16...@gmail.com/

The key difference is that my approach checks the bound
state BEFORE calling pn_socket_bind(), rather than after:

--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -207,12 +207,11 @@ 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;
}

The root cause is that pn_socket_bind() returns -EINVAL
for multiple reasons:

1. address length too short
2. sk_state != TCP_CLOSE (without prior bind)
3. socket already bound <- only intended case

Your fix correctly prevents the crash. However the
ambiguous "if (err != -EINVAL)" path still remains.

By checking pn_port(sobject) BEFORE calling
pn_socket_bind(), this approach:

- eliminates the -EINVAL ambiguity entirely
- removes the special -EINVAL handling path
- makes "already bound" check direct and clear
- simplifies the overall logic flow

Both fixes prevent the crash, but this removes the
underlying ambiguity rather than working around it.

Thoughts? Happy to defer to your patch if maintainers
prefer the minimal change approach.

Thanks,
Deepanshu Kartikey

[Morduan Zang]

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+706f5e...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=706f5eb79044e686c794
Signed-off-by: Morduan Zang <zhang...@uniontech.com>
Signed-off-by: zhanjun <zha...@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;

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

--
2.50.1

[Rémi Denis-Courmont]

Hi,

Le keskiviikkona 22. huhtikuuta 2026, 4.38.07 Itä-Euroopan kesäaika Morduan
Zang a écrit :

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

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


--

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

[Morduan Zang]

Hi Remi,

On Wed, Apr 22, 2026 at 06:18:48PM +0300, Remi Denis-Courmont wrote:
> This could be written as just
> if (err != -EINVAL || unlikely(...)) return err;

Thanks, that reads much better. Folded into v2 and resending now.

Morduan

[Morduan Zang]

Suggested-by: Remi Denis-Courmont <cour...@gmail.com>

Signed-off-by: Morduan Zang <zhang...@uniontech.com>
Signed-off-by: zhanjun <zha...@uniontech.com>
---

Changes in v2:
- Fold the extra port check into the existing -EINVAL test so that
autobind now reads as a single compact "not already bound" guard
using unlikely() (Remi Denis-Courmont).
- Reword the accompanying comment accordingly; no functional change
vs. v1 other than the code-style simplification.

v1: https://lore.kernel.org/all/81A6570B633FF6FE+20260422...@uniontech.com/
---
net/phonet/socket.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 4423d483c630..bbd710d95b97 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -208,9 +208,15 @@ static int pn_socket_autobind(struct socket *sock)

sa.spn_family = AF_PHONET;
err = pn_socket_bind(sock, (struct sockaddr_unsized *)&sa,
sizeof(struct sockaddr_pn));
- if (err != -EINVAL)

+ /*
+ * pn_socket_bind() also returns -EINVAL when sk_state != TCP_CLOSE
+ * without a prior bind, so -EINVAL alone is not sufficient to infer
+ * that the socket was already bound. Only treat it as "already
+ * bound" when the port is non-zero; otherwise propagate the error
+ * instead of crashing the kernel.
+ */
+ if (err != -EINVAL || unlikely(!pn_port(pn_sk(sock->sk)->sobject)))

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