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

[Deepanshu Kartikey <kartikey406@gmail.com>]

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.16987-1-kartikey406@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