| Message ID | 71becb489e51284edf0c11fc15246f4ed4cef5b6.1649337862.git.lucien.xin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e2d88f9ce678cd33763826ae2f0412f181251314 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [PATCHv2,net] sctp: use the correct skb for security_sctp_assoc_request | expand |
On Thu, Apr 7, 2022 at 3:24 PM Xin Long <lucien.xin@gmail.com> wrote: > Yi Chen reported an unexpected sctp connection abort, and it occurred when > COOKIE_ECHO is bundled with DATA Fragment by SCTP HW GSO. As the IP header > is included in chunk->head_skb instead of chunk->skb, it failed to check > IP header version in security_sctp_assoc_request(). > > According to Ondrej, SELinux only looks at IP header (address and IPsec > options) and XFRM state data, and these are all included in head_skb for > SCTP HW GSO packets. So fix it by using head_skb when calling > security_sctp_assoc_request() in processing COOKIE_ECHO. > > v1->v2: > - As Ondrej noticed, chunk->head_skb should also be used for > security_sctp_assoc_established() in sctp_sf_do_5_1E_ca(). > > Fixes: e215dab1c490 ("security: call security_sctp_assoc_request in sctp_sf_do_5_1D_ce") > Reported-by: Yi Chen <yiche@redhat.com> > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/sctp/sm_statefuns.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c > index 7f342bc12735..52edee1322fc 100644 > --- a/net/sctp/sm_statefuns.c > +++ b/net/sctp/sm_statefuns.c > @@ -781,7 +781,7 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net, > } > } > > - if (security_sctp_assoc_request(new_asoc, chunk->skb)) { > + if (security_sctp_assoc_request(new_asoc, chunk->head_skb ?: chunk->skb)) { > sctp_association_free(new_asoc); > return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); > } > @@ -932,7 +932,7 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net, > > /* Set peer label for connection. */ > if (security_sctp_assoc_established((struct sctp_association *)asoc, > - chunk->skb)) > + chunk->head_skb ?: chunk->skb)) > return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); > > /* Verify that the chunk length for the COOKIE-ACK is OK. > @@ -2262,7 +2262,7 @@ enum sctp_disposition sctp_sf_do_5_2_4_dupcook( > } > > /* Update socket peer label if first association. */ > - if (security_sctp_assoc_request(new_asoc, chunk->skb)) { > + if (security_sctp_assoc_request(new_asoc, chunk->head_skb ?: chunk->skb)) { > sctp_association_free(new_asoc); > return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); > } > -- > 2.31.1 FWIW: Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com> Thanks!
On Thu, Apr 07, 2022 at 09:24:22AM -0400, Xin Long wrote: > Yi Chen reported an unexpected sctp connection abort, and it occurred when > COOKIE_ECHO is bundled with DATA Fragment by SCTP HW GSO. As the IP header > is included in chunk->head_skb instead of chunk->skb, it failed to check > IP header version in security_sctp_assoc_request(). > > According to Ondrej, SELinux only looks at IP header (address and IPsec > options) and XFRM state data, and these are all included in head_skb for > SCTP HW GSO packets. So fix it by using head_skb when calling > security_sctp_assoc_request() in processing COOKIE_ECHO. > > v1->v2: > - As Ondrej noticed, chunk->head_skb should also be used for > security_sctp_assoc_established() in sctp_sf_do_5_1E_ca(). > > Fixes: e215dab1c490 ("security: call security_sctp_assoc_request in sctp_sf_do_5_1D_ce") > Reported-by: Yi Chen <yiche@redhat.com> > Signed-off-by: Xin Long <lucien.xin@gmail.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Thu, 7 Apr 2022 09:24:22 -0400 you wrote: > Yi Chen reported an unexpected sctp connection abort, and it occurred when > COOKIE_ECHO is bundled with DATA Fragment by SCTP HW GSO. As the IP header > is included in chunk->head_skb instead of chunk->skb, it failed to check > IP header version in security_sctp_assoc_request(). > > According to Ondrej, SELinux only looks at IP header (address and IPsec > options) and XFRM state data, and these are all included in head_skb for > SCTP HW GSO packets. So fix it by using head_skb when calling > security_sctp_assoc_request() in processing COOKIE_ECHO. > > [...] Here is the summary with links: - [PATCHv2,net] sctp: use the correct skb for security_sctp_assoc_request https://git.kernel.org/netdev/net/c/e2d88f9ce678 You are awesome, thank you!
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 7f342bc12735..52edee1322fc 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -781,7 +781,7 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net, } } - if (security_sctp_assoc_request(new_asoc, chunk->skb)) { + if (security_sctp_assoc_request(new_asoc, chunk->head_skb ?: chunk->skb)) { sctp_association_free(new_asoc); return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); } @@ -932,7 +932,7 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net, /* Set peer label for connection. */ if (security_sctp_assoc_established((struct sctp_association *)asoc, - chunk->skb)) + chunk->head_skb ?: chunk->skb)) return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); /* Verify that the chunk length for the COOKIE-ACK is OK. @@ -2262,7 +2262,7 @@ enum sctp_disposition sctp_sf_do_5_2_4_dupcook( } /* Update socket peer label if first association. */ - if (security_sctp_assoc_request(new_asoc, chunk->skb)) { + if (security_sctp_assoc_request(new_asoc, chunk->head_skb ?: chunk->skb)) { sctp_association_free(new_asoc); return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); }
Yi Chen reported an unexpected sctp connection abort, and it occurred when COOKIE_ECHO is bundled with DATA Fragment by SCTP HW GSO. As the IP header is included in chunk->head_skb instead of chunk->skb, it failed to check IP header version in security_sctp_assoc_request(). According to Ondrej, SELinux only looks at IP header (address and IPsec options) and XFRM state data, and these are all included in head_skb for SCTP HW GSO packets. So fix it by using head_skb when calling security_sctp_assoc_request() in processing COOKIE_ECHO. v1->v2: - As Ondrej noticed, chunk->head_skb should also be used for security_sctp_assoc_established() in sctp_sf_do_5_1E_ca(). Fixes: e215dab1c490 ("security: call security_sctp_assoc_request in sctp_sf_do_5_1D_ce") Reported-by: Yi Chen <yiche@redhat.com> Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/sctp/sm_statefuns.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)