| Message ID | 136c1a7a419341487c504be6d1996928d9d16e02.1604472932.git.lucien.xin@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] sctp: bring inet(6)_skb_parm back to sctp_input_cb | expand |
On Wed, Nov 04, 2020 at 02:55:32PM +0800, Xin Long wrote: > inet(6)_skb_parm was removed from sctp_input_cb by Commit a1dd2cf2f1ae > ("sctp: allow changing transport encap_port by peer packets"), as it > thought sctp_input_cb->header is not used any more in SCTP. > > syzbot reported a crash: > > [ ] BUG: KASAN: use-after-free in decode_session6+0xe7c/0x1580 > [ ] > [ ] Call Trace: > [ ] <IRQ> > [ ] dump_stack+0x107/0x163 > [ ] kasan_report.cold+0x1f/0x37 > [ ] decode_session6+0xe7c/0x1580 > [ ] __xfrm_policy_check+0x2fa/0x2850 > [ ] sctp_rcv+0x12b0/0x2e30 > [ ] sctp6_rcv+0x22/0x40 > [ ] ip6_protocol_deliver_rcu+0x2e8/0x1680 > [ ] ip6_input_finish+0x7f/0x160 > [ ] ip6_input+0x9c/0xd0 > [ ] ipv6_rcv+0x28e/0x3c0 > > It was caused by sctp_input_cb->header/IP6CB(skb) still used in sctp rx > path decode_session6() but some members overwritten by sctp6_rcv(). > > This patch is to fix it by bring inet(6)_skb_parm back to sctp_input_cb > and not overwriting it in sctp4/6_rcv() and sctp_udp_rcv(). > > Reported-by: syzbot+5be8aebb1b7dfa90ef31@syzkaller.appspotmail.com > Fixes: a1dd2cf2f1ae ("sctp: allow changing transport encap_port by peer packets") > Signed-off-by: Xin Long <lucien.xin@gmail.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Thanks Xin.
On Thu, 5 Nov 2020 00:49:09 -0300 Marcelo Ricardo Leitner wrote: > On Wed, Nov 04, 2020 at 02:55:32PM +0800, Xin Long wrote: > > inet(6)_skb_parm was removed from sctp_input_cb by Commit a1dd2cf2f1ae > > ("sctp: allow changing transport encap_port by peer packets"), as it > > thought sctp_input_cb->header is not used any more in SCTP. > > > > syzbot reported a crash: > > > > [ ] BUG: KASAN: use-after-free in decode_session6+0xe7c/0x1580 > > [ ] > > [ ] Call Trace: > > [ ] <IRQ> > > [ ] dump_stack+0x107/0x163 > > [ ] kasan_report.cold+0x1f/0x37 > > [ ] decode_session6+0xe7c/0x1580 > > [ ] __xfrm_policy_check+0x2fa/0x2850 > > [ ] sctp_rcv+0x12b0/0x2e30 > > [ ] sctp6_rcv+0x22/0x40 > > [ ] ip6_protocol_deliver_rcu+0x2e8/0x1680 > > [ ] ip6_input_finish+0x7f/0x160 > > [ ] ip6_input+0x9c/0xd0 > > [ ] ipv6_rcv+0x28e/0x3c0 > > > > It was caused by sctp_input_cb->header/IP6CB(skb) still used in sctp rx > > path decode_session6() but some members overwritten by sctp6_rcv(). > > > > This patch is to fix it by bring inet(6)_skb_parm back to sctp_input_cb > > and not overwriting it in sctp4/6_rcv() and sctp_udp_rcv(). > > > > Reported-by: syzbot+5be8aebb1b7dfa90ef31@syzkaller.appspotmail.com > > Fixes: a1dd2cf2f1ae ("sctp: allow changing transport encap_port by peer packets") > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > > Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Applied, thanks!
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h index 80f7149..1aa5852 100644 --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h @@ -1121,6 +1121,12 @@ static inline void sctp_outq_cork(struct sctp_outq *q) * sctp_input_cb is currently used on rx and sock rx queue */ struct sctp_input_cb { + union { + struct inet_skb_parm h4; +#if IS_ENABLED(CONFIG_IPV6) + struct inet6_skb_parm h6; +#endif + } header; struct sctp_chunk *chunk; struct sctp_af *af; __be16 encap_port; diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c index 814754d..c3e89c7 100644 --- a/net/sctp/ipv6.c +++ b/net/sctp/ipv6.c @@ -1074,7 +1074,7 @@ static struct inet_protosw sctpv6_stream_protosw = { static int sctp6_rcv(struct sk_buff *skb) { - memset(skb->cb, 0, sizeof(skb->cb)); + SCTP_INPUT_CB(skb)->encap_port = 0; return sctp_rcv(skb) ? -1 : 0; } diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index 41f287a..6f2bbfe 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -843,7 +843,6 @@ static int sctp_ctl_sock_init(struct net *net) static int sctp_udp_rcv(struct sock *sk, struct sk_buff *skb) { - memset(skb->cb, 0, sizeof(skb->cb)); SCTP_INPUT_CB(skb)->encap_port = udp_hdr(skb)->source; skb_set_transport_header(skb, sizeof(struct udphdr)); @@ -1163,7 +1162,7 @@ static struct inet_protosw sctp_stream_protosw = { static int sctp4_rcv(struct sk_buff *skb) { - memset(skb->cb, 0, sizeof(skb->cb)); + SCTP_INPUT_CB(skb)->encap_port = 0; return sctp_rcv(skb); }
inet(6)_skb_parm was removed from sctp_input_cb by Commit a1dd2cf2f1ae ("sctp: allow changing transport encap_port by peer packets"), as it thought sctp_input_cb->header is not used any more in SCTP. syzbot reported a crash: [ ] BUG: KASAN: use-after-free in decode_session6+0xe7c/0x1580 [ ] [ ] Call Trace: [ ] <IRQ> [ ] dump_stack+0x107/0x163 [ ] kasan_report.cold+0x1f/0x37 [ ] decode_session6+0xe7c/0x1580 [ ] __xfrm_policy_check+0x2fa/0x2850 [ ] sctp_rcv+0x12b0/0x2e30 [ ] sctp6_rcv+0x22/0x40 [ ] ip6_protocol_deliver_rcu+0x2e8/0x1680 [ ] ip6_input_finish+0x7f/0x160 [ ] ip6_input+0x9c/0xd0 [ ] ipv6_rcv+0x28e/0x3c0 It was caused by sctp_input_cb->header/IP6CB(skb) still used in sctp rx path decode_session6() but some members overwritten by sctp6_rcv(). This patch is to fix it by bring inet(6)_skb_parm back to sctp_input_cb and not overwriting it in sctp4/6_rcv() and sctp_udp_rcv(). Reported-by: syzbot+5be8aebb1b7dfa90ef31@syzkaller.appspotmail.com Fixes: a1dd2cf2f1ae ("sctp: allow changing transport encap_port by peer packets") Signed-off-by: Xin Long <lucien.xin@gmail.com> --- include/net/sctp/structs.h | 6 ++++++ net/sctp/ipv6.c | 2 +- net/sctp/protocol.c | 3 +-- 3 files changed, 8 insertions(+), 3 deletions(-)