* [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown @ 2026-07-16 6:52 luoqing 2026-07-16 6:52 ` [PATCH 2/2] sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling luoqing 2026-07-16 7:42 ` [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown Jagielski, Jedrzej 0 siblings, 2 replies; 6+ messages in thread From: luoqing @ 2026-07-16 6:52 UTC (permalink / raw) To: marcelo.leitner, lucien.xin, davem, edumazet, kuba, pabeni Cc: horms, linux-sctp, netdev, linux-kernel, luoqing From: luoqing <luoqing@kylinos.cn> When sctp_skb_recv_datagram() detects sk->sk_shutdown & RCV_SHUTDOWN, it breaks out of the loop and returns NULL without setting *err. This leaves the error pointer uninitialized or with a stale value, which can confuse callers expecting a clean shutdown indication. Compare with the generic __skb_wait_for_more_packets() in net/core/datagram.c which properly handles shutdown by setting *err = 0. Fix this by setting *err = 0 before breaking when the socket is shut down, indicating an orderly shutdown rather than an error condition. Signed-off-by: luoqing <luoqing@kylinos.cn> --- net/sctp/socket.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index c7b9e325ec1c..ea7050b27715 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -9117,8 +9117,10 @@ 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) { + *err = 0; break; + } /* User doesn't want to wait. */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling 2026-07-16 6:52 [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown luoqing @ 2026-07-16 6:52 ` luoqing 2026-07-16 7:42 ` [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown Jagielski, Jedrzej 1 sibling, 0 replies; 6+ messages in thread From: luoqing @ 2026-07-16 6:52 UTC (permalink / raw) To: marcelo.leitner, lucien.xin, davem, edumazet, kuba, pabeni Cc: horms, linux-sctp, netdev, linux-kernel, luoqing From: luoqing <luoqing@kylinos.cn> When processing AUTH + COOKIE-ECHO packets, if skb_clone fails due to memory pressure, chunk->auth_chunk is set to NULL but chunk->auth is still set to 1. This causes sctp_auth_chunk_verify to skip the AUTH validation (since auth_chunk is NULL), allowing unauthenticated COOKIE-ECHO packets to be accepted. Fix this by only setting chunk->auth = 1 when skb_clone succeeds. Signed-off-by: luoqing <luoqing@kylinos.cn> --- net/sctp/associola.c | 3 ++- net/sctp/endpointola.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/net/sctp/associola.c b/net/sctp/associola.c index 62d3cc155809..e54068305396 100644 --- a/net/sctp/associola.c +++ b/net/sctp/associola.c @@ -999,7 +999,8 @@ static void sctp_assoc_bh_rcv(struct work_struct *work) if (next_hdr->type == SCTP_CID_COOKIE_ECHO) { chunk->auth_chunk = skb_clone(chunk->skb, GFP_ATOMIC); - chunk->auth = 1; + if (chunk->auth_chunk) + chunk->auth = 1; continue; } } diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c index dfb1719275db..3419748c66bc 100644 --- a/net/sctp/endpointola.c +++ b/net/sctp/endpointola.c @@ -368,7 +368,8 @@ static void sctp_endpoint_bh_rcv(struct work_struct *work) if (next_hdr->type == SCTP_CID_COOKIE_ECHO) { chunk->auth_chunk = skb_clone(chunk->skb, GFP_ATOMIC); - chunk->auth = 1; + if (chunk->auth_chunk) + chunk->auth = 1; continue; } } -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown 2026-07-16 6:52 [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown luoqing 2026-07-16 6:52 ` [PATCH 2/2] sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling luoqing @ 2026-07-16 7:42 ` Jagielski, Jedrzej 2026-07-17 8:20 ` [PATCH net v3] sctp: socket: remove unused 'err' parameter from sctp_skb_recv_datagram luoqing 1 sibling, 1 reply; 6+ messages in thread From: Jagielski, Jedrzej @ 2026-07-16 7:42 UTC (permalink / raw) To: luoqing, marcelo.leitner@gmail.com, lucien.xin@gmail.com, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com Cc: horms@kernel.org, linux-sctp@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, luoqing@kylinos.cn From: luoqing <l1138897701@163.com> Sent: Thursday, July 16, 2026 8:53 AM >From: luoqing <luoqing@kylinos.cn> > >When sctp_skb_recv_datagram() detects sk->sk_shutdown & RCV_SHUTDOWN, >it breaks out of the loop and returns NULL without setting *err. >This leaves the error pointer uninitialized or with a stale value, >which can confuse callers expecting a clean shutdown indication. > >Compare with the generic __skb_wait_for_more_packets() in >net/core/datagram.c which properly handles shutdown by setting *err = 0. > >Fix this by setting *err = 0 before breaking when the socket is shut down, >indicating an orderly shutdown rather than an error condition. Hi luoqing patch title should contain target tree fix patches should contain 'fixes' tag and be targeted to net tree please put Name + Surname for the signed-off-by tag > >Signed-off-by: luoqing <luoqing@kylinos.cn> >--- > net/sctp/socket.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/net/sctp/socket.c b/net/sctp/socket.c >index c7b9e325ec1c..ea7050b27715 100644 >--- a/net/sctp/socket.c >+++ b/net/sctp/socket.c >@@ -9117,8 +9117,10 @@ 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) { >+ *err = 0; from what i see any of the sctp_skb_recv_datagram() callers doesn't use @err - so at this point that fix doesn't affect any code i'm wondering if we could just drop @err > break; >+ } > > > /* User doesn't want to wait. */ >-- >2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net v3] sctp: socket: remove unused 'err' parameter from sctp_skb_recv_datagram 2026-07-16 7:42 ` [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown Jagielski, Jedrzej @ 2026-07-17 8:20 ` luoqing 2026-07-17 15:10 ` Xin Long 0 siblings, 1 reply; 6+ messages in thread From: luoqing @ 2026-07-17 8:20 UTC (permalink / raw) To: jedrzej.jagielski Cc: davem, edumazet, horms, kuba, linux-kernel, linux-sctp, lucien.xin, luoqing, marcelo.leitner, netdev, pabeni From: luoqing <luoqing@kylinos.cn> The 'err' parameter in sctp_skb_recv_datagram() is never used by any of its callers. Both sctp_recvmsg() and sctp_ulpevent_read_nxtinfo() pass the address of a local variable but never check its value after the function returns, rendering the parameter completely useless. Remove the unused parameter to simplify the function signature and eliminate dead code. Signed-off-by: luoqing <luoqing@kylinos.cn> --- include/net/sctp/sctp.h | 2 +- net/sctp/socket.c | 10 +++------- net/sctp/ulpevent.c | 3 +-- 3 files changed, 5 insertions(+), 10 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..3804382d78e0 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -2123,7 +2123,7 @@ static int sctp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, goto out; } - skb = sctp_skb_recv_datagram(sk, flags, &err); + skb = sctp_skb_recv_datagram(sk, flags); if (!skb) goto out; @@ -9082,7 +9082,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; @@ -9120,17 +9120,13 @@ struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags, int *err) if (sk->sk_shutdown & RCV_SHUTDOWN) 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; } diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c index 8920ca92a011..8ed51a15c3a4 100644 --- a/net/sctp/ulpevent.c +++ b/net/sctp/ulpevent.c @@ -1061,9 +1061,8 @@ 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); + skb = sctp_skb_recv_datagram(sk, MSG_PEEK | MSG_DONTWAIT); if (skb != NULL) { __sctp_ulpevent_read_nxtinfo(sctp_skb2event(skb), msghdr, skb); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v3] sctp: socket: remove unused 'err' parameter from sctp_skb_recv_datagram 2026-07-17 8:20 ` [PATCH net v3] sctp: socket: remove unused 'err' parameter from sctp_skb_recv_datagram luoqing @ 2026-07-17 15:10 ` Xin Long 2026-07-17 18:19 ` David Laight 0 siblings, 1 reply; 6+ messages in thread From: Xin Long @ 2026-07-17 15:10 UTC (permalink / raw) To: luoqing Cc: jedrzej.jagielski, davem, edumazet, horms, kuba, linux-kernel, linux-sctp, luoqing, marcelo.leitner, netdev, pabeni On Fri, Jul 17, 2026 at 4:21 AM luoqing <l1138897701@163.com> wrote: > > From: luoqing <luoqing@kylinos.cn> > > The 'err' parameter in sctp_skb_recv_datagram() is never used by any > of its callers. Both sctp_recvmsg() and sctp_ulpevent_read_nxtinfo() > pass the address of a local variable but never check its value after > the function returns, rendering the parameter completely useless. > > Remove the unused parameter to simplify the function signature and > eliminate dead code. > > Signed-off-by: luoqing <luoqing@kylinos.cn> > --- > include/net/sctp/sctp.h | 2 +- > net/sctp/socket.c | 10 +++------- > net/sctp/ulpevent.c | 3 +-- > 3 files changed, 5 insertions(+), 10 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..3804382d78e0 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -2123,7 +2123,7 @@ static int sctp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > goto out; > } > > - skb = sctp_skb_recv_datagram(sk, flags, &err); > + skb = sctp_skb_recv_datagram(sk, flags); > if (!skb) > goto out; > > @@ -9082,7 +9082,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; > @@ -9120,17 +9120,13 @@ struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags, int *err) > if (sk->sk_shutdown & RCV_SHUTDOWN) > 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; > } > > diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c > index 8920ca92a011..8ed51a15c3a4 100644 > --- a/net/sctp/ulpevent.c > +++ b/net/sctp/ulpevent.c > @@ -1061,9 +1061,8 @@ 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); > + skb = sctp_skb_recv_datagram(sk, MSG_PEEK | MSG_DONTWAIT); > if (skb != NULL) { > __sctp_ulpevent_read_nxtinfo(sctp_skb2event(skb), > msghdr, skb); > -- > 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; ... out: release_sock(sk); return err; <------ [1] Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v3] sctp: socket: remove unused 'err' parameter from sctp_skb_recv_datagram 2026-07-17 15:10 ` Xin Long @ 2026-07-17 18:19 ` David Laight 0 siblings, 0 replies; 6+ messages in thread From: David Laight @ 2026-07-17 18:19 UTC (permalink / raw) To: Xin Long Cc: luoqing, jedrzej.jagielski, davem, edumazet, horms, kuba, linux-kernel, linux-sctp, luoqing, marcelo.leitner, netdev, pabeni On Fri, 17 Jul 2026 11:10:21 -0400 Xin Long <lucien.xin@gmail.com> wrote: > On Fri, Jul 17, 2026 at 4:21 AM luoqing <l1138897701@163.com> wrote: > > > > From: luoqing <luoqing@kylinos.cn> > > > > The 'err' parameter in sctp_skb_recv_datagram() is never used by any > > of its callers. Both sctp_recvmsg() and sctp_ulpevent_read_nxtinfo() > > pass the address of a local variable but never check its value after > > the function returns, rendering the parameter completely useless. > > > > Remove the unused parameter to simplify the function signature and > > eliminate dead code. > > > > Signed-off-by: luoqing <luoqing@kylinos.cn> > > --- > > include/net/sctp/sctp.h | 2 +- > > net/sctp/socket.c | 10 +++------- > > net/sctp/ulpevent.c | 3 +-- > > 3 files changed, 5 insertions(+), 10 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..3804382d78e0 100644 > > --- a/net/sctp/socket.c > > +++ b/net/sctp/socket.c > > @@ -2123,7 +2123,7 @@ static int sctp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > > goto out; > > } > > > > - skb = sctp_skb_recv_datagram(sk, flags, &err); > > + skb = sctp_skb_recv_datagram(sk, flags); > > if (!skb) > > goto out; > > > > @@ -9082,7 +9082,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; > > @@ -9120,17 +9120,13 @@ struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags, int *err) > > if (sk->sk_shutdown & RCV_SHUTDOWN) > > 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; > > } > > > > diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c > > index 8920ca92a011..8ed51a15c3a4 100644 > > --- a/net/sctp/ulpevent.c > > +++ b/net/sctp/ulpevent.c > > @@ -1061,9 +1061,8 @@ 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); > > + skb = sctp_skb_recv_datagram(sk, MSG_PEEK | MSG_DONTWAIT); > > if (skb != NULL) { > > __sctp_ulpevent_read_nxtinfo(sctp_skb2event(skb), > > msghdr, skb); > > -- > > 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 > ... > > out: > release_sock(sk); > return err; <------ [1] > > Thanks. > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-17 18:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-16 6:52 [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown luoqing 2026-07-16 6:52 ` [PATCH 2/2] sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling luoqing 2026-07-16 7:42 ` [PATCH 1/2] sctp: socket: Fix uninitialized error on socket shutdown Jagielski, Jedrzej 2026-07-17 8:20 ` [PATCH net v3] sctp: socket: remove unused 'err' parameter from sctp_skb_recv_datagram luoqing 2026-07-17 15:10 ` Xin Long 2026-07-17 18:19 ` David Laight
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox