[PATCH net v4] sctp: socket: refactor sctp_skb_recv_datagram to use ERR_PTR

[PATCH net v4] sctp: socket: refactor sctp_skb_recv_datagram to use ERR_PTR

[luoqing]

From: Qing Luo <luoqing@kylinos.cn>

The err output parameter in sctp_skb_recv_datagram() is passed to
callers but never validated, making error reporting unreliable.
Remove it and use ERR_PTR to encode errors directly in the return
value, which is the standard kernel pattern for this case.

Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 include/net/sctp/sctp.h |  2 +-
 net/sctp/socket.c       | 20 ++++++++++----------
 net/sctp/ulpevent.c     |  5 ++---
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index d50c27812504..b86d50d6b146 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -97,7 +97,7 @@ void sctp_sock_rfree(struct sk_buff *skb);
 
 extern struct percpu_counter sctp_sockets_allocated;
 int sctp_asconf_mgmt(struct sctp_sock *, struct sctp_sockaddr_entry *);
-struct sk_buff *sctp_skb_recv_datagram(struct sock *, int, int *);
+struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags);
 
 typedef int (*sctp_callback_t)(struct sctp_endpoint *, struct sctp_transport *, void *);
 void sctp_transport_walk_start(struct rhashtable_iter *iter);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index c7b9e325ec1c..2deaa498e6cf 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2123,9 +2123,11 @@ static int sctp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		goto out;
 	}
 
-	skb = sctp_skb_recv_datagram(sk, flags, &err);
-	if (!skb)
+	skb = sctp_skb_recv_datagram(sk, flags);
+	if (IS_ERR(skb)) {
+		err = PTR_ERR(skb);
 		goto out;
+	}
 
 	/* Get the total length of the skb including any skb's in the
 	 * frag_list.
@@ -9082,7 +9084,7 @@ static int sctp_wait_for_packet(struct sock *sk, int *err, long *timeo_p)
  * Note: This is pretty much the same routine as in core/datagram.c
  * with a few changes to make lksctp work.
  */
-struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags, int *err)
+struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags)
 {
 	int error;
 	struct sk_buff *skb;
@@ -9117,21 +9119,19 @@ struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags, int *err)
 		if (error)
 			goto no_packet;
 
-		if (sk->sk_shutdown & RCV_SHUTDOWN)
+		if (sk->sk_shutdown & RCV_SHUTDOWN) {
+			error = 0;
 			break;
-
+		}
 
 		/* User doesn't want to wait.  */
 		error = -EAGAIN;
 		if (!timeo)
 			goto no_packet;
-	} while (sctp_wait_for_packet(sk, err, &timeo) == 0);
-
-	return NULL;
+	} while (sctp_wait_for_packet(sk, &error, &timeo) == 0);
 
 no_packet:
-	*err = error;
-	return NULL;
+	return ERR_PTR(error);
 }
 
 /* If sndbuf has changed, wake up per association sndbuf waiters.  */
diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
index 8920ca92a011..21ae0adbaeef 100644
--- a/net/sctp/ulpevent.c
+++ b/net/sctp/ulpevent.c
@@ -1061,10 +1061,9 @@ void sctp_ulpevent_read_nxtinfo(const struct sctp_ulpevent *event,
 				struct sock *sk)
 {
 	struct sk_buff *skb;
-	int err;
 
-	skb = sctp_skb_recv_datagram(sk, MSG_PEEK | MSG_DONTWAIT, &err);
-	if (skb != NULL) {
+	skb = sctp_skb_recv_datagram(sk, MSG_PEEK | MSG_DONTWAIT);
+	if (!IS_ERR_OR_NULL(skb)) {
 		__sctp_ulpevent_read_nxtinfo(sctp_skb2event(skb),
 					     msghdr, skb);
 		/* Just release refcount here. */
-- 
2.25.1
>> I think it's used at [1] in sctp_recvmsg():
>>
>>         skb = sctp_skb_recv_datagram(sk, flags, &err);
>>         if (!skb)
>>                 goto out;
> Would it make more sense to ERR_PTR() etc ?
>
>
> 	David

Yes, you are right. The current implementation returns a negative error
code directly via err, but it would be cleaner to use ERR_PTR() to
unify the error path with other datagram receivers.

I will refactor this part in v4:
>
>
>> ...
>>
>> out:
>>         release_sock(sk);
>>         return err;   <------ [1]

[syzbot ci] Re: sctp: socket: refactor sctp_skb_recv_datagram to use ERR_PTR

[syzbot ci]

syzbot ci has tested the following series

[v4] sctp: socket: refactor sctp_skb_recv_datagram to use ERR_PTR
https://lore.kernel.org/all/20260720082145.1072547-1-l1138897701@163.com
* [PATCH net v4] sctp: socket: refactor sctp_skb_recv_datagram to use ERR_PTR

and found the following issue:
general protection fault in sctp_recvmsg

Full report is available here:
https://ci.syzbot.org/series/f979ded9-9743-486d-b911-69a904c9f4ad

***

general protection fault in sctp_recvmsg

tree:      net
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net.git
base:      e13caf1c26587434f0b768193100440939c0fb91
arch:      amd64
compiler:  Debian clang version 22.1.6 (++20260514074242+fc4aad7b5db3-1~exp1~20260514074407.73), Debian LLD 22.1.6
config:    https://ci.syzbot.org/builds/e4b39daf-f862-498e-8aeb-c22b5a168551/config
syz repro: https://ci.syzbot.org/findings/1893a3b4-56e4-49d0-9865-4ff919f027b3/syz_repro

Oops: general protection fault, probably for non-canonical address 0xdffffc000000000e: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000070-0x0000000000000077]
CPU: 0 UID: 0 PID: 5830 Comm: syz.2.19 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:sctp_recvmsg+0x28d/0xd60 net/sctp/socket.c:2135
Code: ff 72 0d e8 35 6a d6 f6 41 89 ee e9 da 06 00 00 4c 89 7c 24 08 48 8d 7d 70 48 89 f8 48 c1 e8 03 48 b9 00 00 00 00 00 fc ff df <0f> b6 04 08 84 c0 0f 85 2f 08 00 00 4c 63 7d 70 4d 39 fe 4d 0f 43
RSP: 0018:ffffc90003a8f838 EFLAGS: 00010202
RAX: 000000000000000e RBX: ffff888115ad1c80 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000070
RBP: 0000000000000000 R08: 1ffff92000751ef4 R09: 1ffff11022b5a460
R10: dffffc0000000000 R11: fffffbfff206639f R12: ffff888115ad22f0
R13: ffffc90003a8fd00 R14: 0000000000000000 R15: 0000000000000300
FS:  00007facf6b766c0(0000) GS:ffff88818dc17000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00002000000055c0 CR3: 000000010a812000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 sock_recvmsg_nosec net/socket.c:1128 [inline]
 sock_recvmsg+0x166/0x1e0 net/socket.c:1148
 ____sys_recvmsg+0x1e6/0x4a0 net/socket.c:2904
 ___sys_recvmsg+0x213/0x5a0 net/socket.c:2946
 do_recvmmsg+0x31a/0x7f0 net/socket.c:3041
 __sys_recvmmsg net/socket.c:3115 [inline]
 __do_sys_recvmmsg net/socket.c:3138 [inline]
 __se_sys_recvmmsg net/socket.c:3131 [inline]
 __x64_sys_recvmmsg+0x198/0x250 net/socket.c:3131
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7facf5d9ce59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007facf6b76028 EFLAGS: 00000246 ORIG_RAX: 000000000000012b
RAX: ffffffffffffffda RBX: 00007facf6015fa0 RCX: 00007facf5d9ce59
RDX: 000000000400023c RSI: 00002000000055c0 RDI: 0000000000000004
RBP: 00007facf5e32e6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000300 R11: 0000000000000246 R12: 0000000000000000
R13: 00007facf6016038 R14: 00007facf6015fa0 R15: 00007ffcc05d60d8
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:sctp_recvmsg+0x28d/0xd60 net/sctp/socket.c:2135
Code: ff 72 0d e8 35 6a d6 f6 41 89 ee e9 da 06 00 00 4c 89 7c 24 08 48 8d 7d 70 48 89 f8 48 c1 e8 03 48 b9 00 00 00 00 00 fc ff df <0f> b6 04 08 84 c0 0f 85 2f 08 00 00 4c 63 7d 70 4d 39 fe 4d 0f 43
RSP: 0018:ffffc90003a8f838 EFLAGS: 00010202
RAX: 000000000000000e RBX: ffff888115ad1c80 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000070
RBP: 0000000000000000 R08: 1ffff92000751ef4 R09: 1ffff11022b5a460
R10: dffffc0000000000 R11: fffffbfff206639f R12: ffff888115ad22f0
R13: ffffc90003a8fd00 R14: 0000000000000000 R15: 0000000000000300
FS:  00007facf6b766c0(0000) GS:ffff88818dc17000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00002000000055c0 CR3: 000000010a812000 CR4: 00000000000006f0
----------------
Code disassembly (best guess):
   0:	ff 72 0d             	push   0xd(%rdx)
   3:	e8 35 6a d6 f6       	call   0xf6d66a3d
   8:	41 89 ee             	mov    %ebp,%r14d
   b:	e9 da 06 00 00       	jmp    0x6ea
  10:	4c 89 7c 24 08       	mov    %r15,0x8(%rsp)
  15:	48 8d 7d 70          	lea    0x70(%rbp),%rdi
  19:	48 89 f8             	mov    %rdi,%rax
  1c:	48 c1 e8 03          	shr    $0x3,%rax
  20:	48 b9 00 00 00 00 00 	movabs $0xdffffc0000000000,%rcx
  27:	fc ff df
* 2a:	0f b6 04 08          	movzbl (%rax,%rcx,1),%eax <-- trapping instruction
  2e:	84 c0                	test   %al,%al
  30:	0f 85 2f 08 00 00    	jne    0x865
  36:	4c 63 7d 70          	movslq 0x70(%rbp),%r15
  3a:	4d 39 fe             	cmp    %r15,%r14
  3d:	4d                   	rex.WRB
  3e:	0f                   	.byte 0xf
  3f:	43                   	rex.XB


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a patch for this bug, please reply with `#syz test`
(should be on a separate line).

The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.