diff mbox series

xfrm: fix bug with DSCP copy to v6 from v4 tunnel

Message ID 20230126102933.1245451-1-chopps@labn.net (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series xfrm: fix bug with DSCP copy to v6 from v4 tunnel | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 5 this patch: 5
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 1 this patch: 1
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes fail Problems with Fixes tag: 1
netdev/build_allmodconfig_warn success Errors and warnings before: 5 this patch: 5
netdev/checkpatch warning WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 227620e29509 ("[IPSEC]: Separate inner/outer mode processing on input")'
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Christian Hopps Jan. 26, 2023, 10:29 a.m. UTC
When copying the DSCP bits for decap-dscp into IPv6 don't assume the
outer encap is always IPv6. Instead, as with the inner IPv4 case, copy
the DSCP bits from the correctly saved "tos" value in the control block.

fixes: 227620e29509 ("[IPSEC]: Separate inner/outer mode processing on input")

Signed-off-by: Christian Hopps <chopps@labn.net>
---
 net/xfrm/xfrm_input.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Herbert Xu Jan. 27, 2023, 10:10 a.m. UTC | #1
On Thu, Jan 26, 2023 at 05:29:34AM -0500, Christian Hopps wrote:
> When copying the DSCP bits for decap-dscp into IPv6 don't assume the
> outer encap is always IPv6. Instead, as with the inner IPv4 case, copy
> the DSCP bits from the correctly saved "tos" value in the control block.
> 
> fixes: 227620e29509 ("[IPSEC]: Separate inner/outer mode processing on input")

The broken code apparently came from

commit b3284df1c86f7ac078dcb8fb250fe3d6437e740c
Author: Florian Westphal <fw@strlen.de>
Date:   Fri Mar 29 21:16:28 2019 +0100

    xfrm: remove input2 indirection from xfrm_mode

Please fix the Fixes header.

Thanks,
Christian Hopps Jan. 27, 2023, 12:31 p.m. UTC | #2
Herbert Xu <herbert@gondor.apana.org.au> writes:

> On Thu, Jan 26, 2023 at 05:29:34AM -0500, Christian Hopps wrote:
>> When copying the DSCP bits for decap-dscp into IPv6 don't assume the
>> outer encap is always IPv6. Instead, as with the inner IPv4 case, copy
>> the DSCP bits from the correctly saved "tos" value in the control block.
>>
>> fixes: 227620e29509 ("[IPSEC]: Separate inner/outer mode processing on input")
>
> The broken code apparently came from
>
> commit b3284df1c86f7ac078dcb8fb250fe3d6437e740c
> Author: Florian Westphal <fw@strlen.de>
> Date:   Fri Mar 29 21:16:28 2019 +0100
>
>     xfrm: remove input2 indirection from xfrm_mode
>
> Please fix the Fixes header.

Yes that's what the immediate git blame points at; however, that code was copied from net/ipv6/xfrm6_mode_tunnel.c:xfrm6_tunnel_input() and that code arrived in:

    b59f45d0b2878 ("[IPSEC] xfrm: Abstract out encapsulation modes")

Originally this code using a different sk_buff layout was from the initial git repo checkin.

    1da177e4c3f41 ("Linux-2.6.12-rc2")

Why don't I just remove the fixes line? I didn't want to include it initially anyway.

Thanks,
Chris.

>
> Thanks,
Herbert Xu Jan. 28, 2023, 1:41 a.m. UTC | #3
On Fri, Jan 27, 2023 at 07:31:54AM -0500, Christian Hopps wrote:
>
> Yes that's what the immediate git blame points at; however, that code was copied from net/ipv6/xfrm6_mode_tunnel.c:xfrm6_tunnel_input() and that code arrived in:
> 
>    b59f45d0b2878 ("[IPSEC] xfrm: Abstract out encapsulation modes")
> 
> Originally this code using a different sk_buff layout was from the initial git repo checkin.
> 
>    1da177e4c3f41 ("Linux-2.6.12-rc2")
> 
> Why don't I just remove the fixes line? I didn't want to include it initially anyway.

On closer inspection my patch was definitely buggy in that it would
place some random value in the DSCP field.  Previously the code
simply didn't copy the TOS value across.

Steffen, keeping the Fixes header is fine by me.

Thanks,
diff mbox series

Patch

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index c06e54a10540..436d29640ac2 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -279,8 +279,7 @@  static int xfrm6_remove_tunnel_encap(struct xfrm_state *x, struct sk_buff *skb)
 		goto out;
 
 	if (x->props.flags & XFRM_STATE_DECAP_DSCP)
-		ipv6_copy_dscp(ipv6_get_dsfield(ipv6_hdr(skb)),
-			       ipipv6_hdr(skb));
+		ipv6_copy_dscp(XFRM_MODE_SKB_CB(skb)->tos, ipipv6_hdr(skb));
 	if (!(x->props.flags & XFRM_STATE_NOECN))
 		ipip6_ecn_decapsulate(skb);