Netdev List
 help / color / mirror / Atom feed
* [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
@ 2026-05-25 20:11 Kacper Kokot
  2026-05-25 21:28 ` Florian Westphal
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Kacper Kokot @ 2026-05-25 20:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Florian Westphal, Phil Sutter, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	netfilter-devel, coreteam, netdev, linux-kernel
  Cc: stable, Kacper Kokot

Padding TCP options with NOPs is optional, so it is legal to send an
MSS option that is not aligned to a word boundary and therefore not
aligned for checksum calculation. The current TCPMSS target is not
robust to this: when the MSS option is unaligned it produces an
invalid checksum, and the packet is dropped.

When the changed word is not aligned, the modified bytes straddle two
checksum words, and using the standard incremental update helper
(which assumes alignment) produces an invalid checksum:

    | w1     | w2     |
OLD |  a  b  |  c  d  |
NEW |  a  b' |  c' d  |

Since b' and c' sit across w1 and w2, we could compute the incremental
checksum in two operations by recalculating w1 and then w2:

    C' = C - w1 + w1' - w2 + w2'

But working it out:

    C' = C - w1 - w2 + w1' + w2'
       = C - (a * 2^8 + b)  - (c * 2^8 + d)
           + (a * 2^8 + b') + (c' * 2^8 + d)
       = C + 2^8 * (a - a + c' - c) + (b' - b + d - d)
       = C + 2^8 * (c' - c) + (b' - b)
       = C - (2^8 * c + b) + (2^8 * c' + b')

So an unaligned incremental checksum can be done in a single operation
by byteswapping the changed bytes before passing them to the helper.
This patch implements that trick for unaligned MSS option updates.

Signed-off-by: Kacper Kokot <kacper.kokot.44@gmail.com>
---
Reproduction script

  #!/usr/bin/env python3
  import argparse
  from scapy.all import *
  
  parser = argparse.ArgumentParser()
  parser.add_argument("target_ip")
  parser.add_argument("target_port", type=int)
  args = parser.parse_args()
  
  def try_handshake(options):
    sport = RandShort()
    ip = IP(dst=args.target_ip)
  
    syn = TCP(
      sport=sport,
      dport=args.target_port,
      flags="S",
      seq=1000,
      options=options
    )
  
    synack = sr1(ip/syn, timeout=2)
  
    print("SYNACK", synack)
    if synack and synack.haslayer(TCP) and synack[TCP].flags == 0x12:
      ack = TCP(
          sport=sport,
          dport=args.target_port,
          flags="R",
          seq=syn.seq + 1,
          ack=synack.seq + 1,
      )
  
      send(ip/ack)
      print("SYN-ACK received")
    else:
      print("No SYN-ACK received")
  
  print("\n>>> MSS Aligned")
  try_handshake([
      ('MSS', 1460),
      ("NOP", None),
      ("NOP", None),
      ('SAckOK', b''),
      ('Timestamp', (12345, 0)),
      ('WScale', 7)
  ])
  
  print("\n>>> MSS Misaligned")
  try_handshake([
      ("NOP", None),
      ('MSS', 1460),
      ("NOP", None),
      ('SAckOK', b''),
      ('Timestamp', (12345, 0)),
      ('WScale', 7)
  ])

A script to reproduce:

  #!/usr/bin/env python3
  import argparse
  from scapy.all import *

  parser = argparse.ArgumentParser()
  parser.add_argument("target_ip")
  parser.add_argument("target_port", type=int)
  args = parser.parse_args()

  mss_aligned_tcp_options = [
      ('MSS', 1460),
      ("NOP", None),
      ("NOP", None),
      ('SAckOK', b''),
      ('Timestamp', (12345, 0)),
      ('WScale', 7)
  ]

  mss_misaligned_tcp_options = [
      ("NOP", None),
      ('MSS', 1460),
      ("NOP", None),
      ('SAckOK', b''),
      ('Timestamp', (12345, 0)),
      ('WScale', 7)
  ]

  def try_handshake(options):
    sport = RandShort()
    ip = IP(dst=args.target_ip)

    syn = TCP(
      sport=sport,
      dport=args.target_port,
      flags="S",
      seq=1000,
      options=options
    )

    synack = sr1(ip/syn, timeout=2)

    print("SYNACK", synack)

    if synack and synack.haslayer(TCP) and synack[TCP].flags == 0x12:
      ack = TCP(
          sport=sport,
          dport=args.target_port,
          flags="R",
          seq=syn.seq + 1,
          ack=synack.seq + 1,
      )

      send(ip/ack)
      print("SYN-ACK response")
    else:
      print("No SYN-ACK received")

  print("\n>>> MSS Aligned")
  try_handshake(mss_aligned_tcp_options)
  print("\n>>> MSS Misaligned")
  try_handshake(mss_misaligned_tcp_options)

 net/netfilter/xt_TCPMSS.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 80e1634bc51f..8e409858505d 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -117,6 +117,7 @@ tcpmss_mangle_packet(struct sk_buff *skb,
 	for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += optlen(opt, i)) {
 		if (opt[i] == TCPOPT_MSS && opt[i+1] == TCPOLEN_MSS) {
 			u_int16_t oldmss;
+			u16 csum_oldmss, csum_newmss;
 
 			oldmss = (opt[i+2] << 8) | opt[i+3];
 
@@ -130,8 +131,19 @@ tcpmss_mangle_packet(struct sk_buff *skb,
 			opt[i+2] = (newmss & 0xff00) >> 8;
 			opt[i+3] = newmss & 0x00ff;
 
+			csum_oldmss = htons(oldmss);
+			csum_newmss = htons(newmss);
+
+			/* MSS may be unaligned; fix up the incremental checksum
+			 * to avoid an invalid checksum and a dropped packet.
+			 */
+			if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {
+				csum_oldmss = swab16(csum_oldmss);
+				csum_newmss = swab16(csum_newmss);
+			}
+
 			inet_proto_csum_replace2(&tcph->check, skb,
-						 htons(oldmss), htons(newmss),
+						 csum_oldmss, csum_newmss,
 						 false);
 			return 0;
 		}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
  2026-05-25 20:11 [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned Kacper Kokot
@ 2026-05-25 21:28 ` Florian Westphal
  2026-05-25 21:44   ` Kacper Kokot
  2026-05-25 22:08   ` Fernando Fernandez Mancera
  2026-05-26 13:50 ` kernel test robot
  2026-05-26 16:46 ` kernel test robot
  2 siblings, 2 replies; 8+ messages in thread
From: Florian Westphal @ 2026-05-25 21:28 UTC (permalink / raw)
  To: Kacper Kokot
  Cc: Pablo Neira Ayuso, Phil Sutter, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netfilter-devel,
	coreteam, netdev, linux-kernel, stable

Kacper Kokot <kacper.kokot.44@gmail.com> wrote:
> Padding TCP options with NOPs is optional, so it is legal to send an
> MSS option that is not aligned to a word boundary and therefore not
> aligned for checksum calculation. The current TCPMSS target is not
> robust to this: when the MSS option is unaligned it produces an
> invalid checksum, and the packet is dropped.

Is this an actual, real world bug?  This code is 20+ years old, all that
this hints at is that they are always aligned in reality?

(Not disputing theoretical problem).

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
  2026-05-25 21:28 ` Florian Westphal
@ 2026-05-25 21:44   ` Kacper Kokot
  2026-05-25 22:08   ` Fernando Fernandez Mancera
  1 sibling, 0 replies; 8+ messages in thread
From: Kacper Kokot @ 2026-05-25 21:44 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Pablo Neira Ayuso, Phil Sutter, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netfilter-devel,
	coreteam, netdev, linux-kernel, stable

> Is this an actual, real world bug?  This code is 20+ years old, all that
> this hints at is that they are always aligned in reality?

No, as far as I know it's theoretical - I just stumbled on it while
debugging something else.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
  2026-05-25 21:28 ` Florian Westphal
  2026-05-25 21:44   ` Kacper Kokot
@ 2026-05-25 22:08   ` Fernando Fernandez Mancera
  2026-05-26  9:31     ` David Laight
  1 sibling, 1 reply; 8+ messages in thread
From: Fernando Fernandez Mancera @ 2026-05-25 22:08 UTC (permalink / raw)
  To: Florian Westphal, Kacper Kokot
  Cc: Pablo Neira Ayuso, Phil Sutter, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netfilter-devel,
	coreteam, netdev, linux-kernel, stable

On 5/25/26 11:28 PM, Florian Westphal wrote:
> Kacper Kokot <kacper.kokot.44@gmail.com> wrote:
>> Padding TCP options with NOPs is optional, so it is legal to send an
>> MSS option that is not aligned to a word boundary and therefore not
>> aligned for checksum calculation. The current TCPMSS target is not
>> robust to this: when the MSS option is unaligned it produces an
>> invalid checksum, and the packet is dropped.
> 
> Is this an actual, real world bug?  This code is 20+ years old, all that
> this hints at is that they are always aligned in reality?
> 

AFAICS, these issues are not present in real environments as MSS option 
is placed at the beginning of the options block making it aligned by 
default usually.

I would say this is more for correctness. I wonder, if we are touching 
this code, we could use the opportunity to make it use 
get_unaligned_be16() instead.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
  2026-05-25 22:08   ` Fernando Fernandez Mancera
@ 2026-05-26  9:31     ` David Laight
  0 siblings, 0 replies; 8+ messages in thread
From: David Laight @ 2026-05-26  9:31 UTC (permalink / raw)
  To: Fernando Fernandez Mancera
  Cc: Florian Westphal, Kacper Kokot, Pablo Neira Ayuso, Phil Sutter,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netfilter-devel, coreteam, netdev, linux-kernel,
	stable

On Tue, 26 May 2026 00:08:15 +0200
Fernando Fernandez Mancera <fmancera@suse.de> wrote:

> On 5/25/26 11:28 PM, Florian Westphal wrote:
> > Kacper Kokot <kacper.kokot.44@gmail.com> wrote:  
> >> Padding TCP options with NOPs is optional, so it is legal to send an
> >> MSS option that is not aligned to a word boundary and therefore not
> >> aligned for checksum calculation. The current TCPMSS target is not
> >> robust to this: when the MSS option is unaligned it produces an
> >> invalid checksum, and the packet is dropped.  
> > 
> > Is this an actual, real world bug?  This code is 20+ years old, all that
> > this hints at is that they are always aligned in reality?
> >   
> 
> AFAICS, these issues are not present in real environments as MSS option 
> is placed at the beginning of the options block making it aligned by 
> default usually.
> 
> I would say this is more for correctness. I wonder, if we are touching 
> this code, we could use the opportunity to make it use 
> get_unaligned_be16() instead.

gcc and clang convert x[0] << 8 | x[1] (etc) to the appropriate single
instruction (and maybe byteswap) on cpu that support misaligned accesses.
So there is little to gain from doing it any other way.

-- David

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
  2026-05-25 20:11 [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned Kacper Kokot
  2026-05-25 21:28 ` Florian Westphal
@ 2026-05-26 13:50 ` kernel test robot
  2026-05-26 15:18   ` David Laight
  2026-05-26 16:46 ` kernel test robot
  2 siblings, 1 reply; 8+ messages in thread
From: kernel test robot @ 2026-05-26 13:50 UTC (permalink / raw)
  To: Kacper Kokot, Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netfilter-devel, coreteam, linux-kernel
  Cc: oe-kbuild-all, netdev, stable, Kacper Kokot

Hi Kacper,

kernel test robot noticed the following build warnings:

[auto build test WARNING on nf-next/main]
[also build test WARNING on netfilter-nf/main linus/master v6.16-rc1 next-20260525]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kacper-Kokot/netfilter-TCPMSS-fix-dropped-packets-when-MSS-option-is-unaligned/20260526-041308
base:   https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git main
patch link:    https://lore.kernel.org/r/20260525201116.407338-2-kacper.kokot.44%40gmail.com
patch subject: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
config: x86_64-rhel-9.4-ltp (https://download.01.org/0day-ci/archive/20260526/202605261527.v5NoRvES-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260526/202605261527.v5NoRvES-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605261527.v5NoRvES-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/netfilter/xt_TCPMSS.c: In function 'tcpmss_mangle_packet':
>> net/netfilter/xt_TCPMSS.c:140:66: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
     140 |                         if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {
         |                                                                  ^


vim +140 net/netfilter/xt_TCPMSS.c

    69	
    70	static int
    71	tcpmss_mangle_packet(struct sk_buff *skb,
    72			     const struct xt_action_param *par,
    73			     unsigned int family,
    74			     unsigned int tcphoff,
    75			     unsigned int minlen)
    76	{
    77		const struct xt_tcpmss_info *info = par->targinfo;
    78		struct tcphdr *tcph;
    79		int len, tcp_hdrlen;
    80		unsigned int i;
    81		__be16 oldval;
    82		u16 newmss;
    83		u8 *opt;
    84	
    85		/* This is a fragment, no TCP header is available */
    86		if (par->fragoff != 0)
    87			return 0;
    88	
    89		if (skb_ensure_writable(skb, skb->len))
    90			return -1;
    91	
    92		len = skb->len - tcphoff;
    93		if (len < (int)sizeof(struct tcphdr))
    94			return -1;
    95	
    96		tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff);
    97		tcp_hdrlen = tcph->doff * 4;
    98	
    99		if (len < tcp_hdrlen || tcp_hdrlen < sizeof(struct tcphdr))
   100			return -1;
   101	
   102		if (info->mss == XT_TCPMSS_CLAMP_PMTU) {
   103			struct net *net = xt_net(par);
   104			unsigned int in_mtu = tcpmss_reverse_mtu(net, skb, family);
   105			unsigned int min_mtu = min(dst_mtu(skb_dst(skb)), in_mtu);
   106	
   107			if (min_mtu <= minlen) {
   108				net_err_ratelimited("unknown or invalid path-MTU (%u)\n",
   109						    min_mtu);
   110				return -1;
   111			}
   112			newmss = min_mtu - minlen;
   113		} else
   114			newmss = info->mss;
   115	
   116		opt = (u_int8_t *)tcph;
   117		for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += optlen(opt, i)) {
   118			if (opt[i] == TCPOPT_MSS && opt[i+1] == TCPOLEN_MSS) {
   119				u_int16_t oldmss;
   120				u16 csum_oldmss, csum_newmss;
   121	
   122				oldmss = (opt[i+2] << 8) | opt[i+3];
   123	
   124				/* Never increase MSS, even when setting it, as
   125				 * doing so results in problems for hosts that rely
   126				 * on MSS being set correctly.
   127				 */
   128				if (oldmss <= newmss)
   129					return 0;
   130	
   131				opt[i+2] = (newmss & 0xff00) >> 8;
   132				opt[i+3] = newmss & 0x00ff;
   133	
   134				csum_oldmss = htons(oldmss);
   135				csum_newmss = htons(newmss);
   136	
   137				/* MSS may be unaligned; fix up the incremental checksum
   138				 * to avoid an invalid checksum and a dropped packet.
   139				 */
 > 140				if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {
   141					csum_oldmss = swab16(csum_oldmss);
   142					csum_newmss = swab16(csum_newmss);
   143				}
   144	
   145				inet_proto_csum_replace2(&tcph->check, skb,
   146							 csum_oldmss, csum_newmss,
   147							 false);
   148				return 0;
   149			}
   150		}
   151	
   152		/* There is data after the header so the option can't be added
   153		 * without moving it, and doing so may make the SYN packet
   154		 * itself too large. Accept the packet unmodified instead.
   155		 */
   156		if (len > tcp_hdrlen)
   157			return 0;
   158	
   159		/* tcph->doff has 4 bits, do not wrap it to 0 */
   160		if (tcp_hdrlen >= 15 * 4)
   161			return 0;
   162	
   163		/*
   164		 * MSS Option not found ?! add it..
   165		 */
   166		if (skb_tailroom(skb) < TCPOLEN_MSS) {
   167			if (pskb_expand_head(skb, 0,
   168					     TCPOLEN_MSS - skb_tailroom(skb),
   169					     GFP_ATOMIC))
   170				return -1;
   171			tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff);
   172		}
   173	
   174		skb_put(skb, TCPOLEN_MSS);
   175	
   176		/*
   177		 * IPv4: RFC 1122 states "If an MSS option is not received at
   178		 * connection setup, TCP MUST assume a default send MSS of 536".
   179		 * IPv6: RFC 2460 states IPv6 has a minimum MTU of 1280 and a minimum
   180		 * length IPv6 header of 60, ergo the default MSS value is 1220
   181		 * Since no MSS was provided, we must use the default values
   182		 */
   183		if (xt_family(par) == NFPROTO_IPV4)
   184			newmss = min(newmss, (u16)536);
   185		else
   186			newmss = min(newmss, (u16)1220);
   187	
   188		opt = (u_int8_t *)tcph + sizeof(struct tcphdr);
   189		memmove(opt + TCPOLEN_MSS, opt, len - sizeof(struct tcphdr));
   190	
   191		inet_proto_csum_replace2(&tcph->check, skb,
   192					 htons(len), htons(len + TCPOLEN_MSS), true);
   193		opt[0] = TCPOPT_MSS;
   194		opt[1] = TCPOLEN_MSS;
   195		opt[2] = (newmss & 0xff00) >> 8;
   196		opt[3] = newmss & 0x00ff;
   197	
   198		inet_proto_csum_replace4(&tcph->check, skb, 0, *((__be32 *)opt), false);
   199	
   200		oldval = ((__be16 *)tcph)[6];
   201		tcph->doff += TCPOLEN_MSS/4;
   202		inet_proto_csum_replace2(&tcph->check, skb,
   203					 oldval, ((__be16 *)tcph)[6], false);
   204		return TCPOLEN_MSS;
   205	}
   206	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
  2026-05-26 13:50 ` kernel test robot
@ 2026-05-26 15:18   ` David Laight
  0 siblings, 0 replies; 8+ messages in thread
From: David Laight @ 2026-05-26 15:18 UTC (permalink / raw)
  To: kernel test robot
  Cc: Kacper Kokot, Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netfilter-devel, coreteam, linux-kernel,
	oe-kbuild-all, netdev, stable

On Tue, 26 May 2026 15:50:00 +0200
kernel test robot <lkp@intel.com> wrote:

> Hi Kacper,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on nf-next/main]
> [also build test WARNING on netfilter-nf/main linus/master v6.16-rc1 next-20260525]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Kacper-Kokot/netfilter-TCPMSS-fix-dropped-packets-when-MSS-option-is-unaligned/20260526-041308
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git main
> patch link:    https://lore.kernel.org/r/20260525201116.407338-2-kacper.kokot.44%40gmail.com
> patch subject: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
> config: x86_64-rhel-9.4-ltp (https://download.01.org/0day-ci/archive/20260526/202605261527.v5NoRvES-lkp@intel.com/config)
> compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260526/202605261527.v5NoRvES-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202605261527.v5NoRvES-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    net/netfilter/xt_TCPMSS.c: In function 'tcpmss_mangle_packet':
> >> net/netfilter/xt_TCPMSS.c:140:66: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]  
>      140 |                         if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {
>          |                                                                  ^

and, of course, the code works fine because 0x1 != 0 is 1.

K (or maybe R) said that with hindsight they should have corrected the
priority of & and | when they added && and || and just fixed all the
existing code so it still worked.

-- David

> 
> 
> vim +140 net/netfilter/xt_TCPMSS.c
> 
>     69	
>     70	static int
>     71	tcpmss_mangle_packet(struct sk_buff *skb,
>     72			     const struct xt_action_param *par,
>     73			     unsigned int family,
>     74			     unsigned int tcphoff,
>     75			     unsigned int minlen)
>     76	{
>     77		const struct xt_tcpmss_info *info = par->targinfo;
>     78		struct tcphdr *tcph;
>     79		int len, tcp_hdrlen;
>     80		unsigned int i;
>     81		__be16 oldval;
>     82		u16 newmss;
>     83		u8 *opt;
>     84	
>     85		/* This is a fragment, no TCP header is available */
>     86		if (par->fragoff != 0)
>     87			return 0;
>     88	
>     89		if (skb_ensure_writable(skb, skb->len))
>     90			return -1;
>     91	
>     92		len = skb->len - tcphoff;
>     93		if (len < (int)sizeof(struct tcphdr))
>     94			return -1;
>     95	
>     96		tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff);
>     97		tcp_hdrlen = tcph->doff * 4;
>     98	
>     99		if (len < tcp_hdrlen || tcp_hdrlen < sizeof(struct tcphdr))
>    100			return -1;
>    101	
>    102		if (info->mss == XT_TCPMSS_CLAMP_PMTU) {
>    103			struct net *net = xt_net(par);
>    104			unsigned int in_mtu = tcpmss_reverse_mtu(net, skb, family);
>    105			unsigned int min_mtu = min(dst_mtu(skb_dst(skb)), in_mtu);
>    106	
>    107			if (min_mtu <= minlen) {
>    108				net_err_ratelimited("unknown or invalid path-MTU (%u)\n",
>    109						    min_mtu);
>    110				return -1;
>    111			}
>    112			newmss = min_mtu - minlen;
>    113		} else
>    114			newmss = info->mss;
>    115	
>    116		opt = (u_int8_t *)tcph;
>    117		for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += optlen(opt, i)) {
>    118			if (opt[i] == TCPOPT_MSS && opt[i+1] == TCPOLEN_MSS) {
>    119				u_int16_t oldmss;
>    120				u16 csum_oldmss, csum_newmss;
>    121	
>    122				oldmss = (opt[i+2] << 8) | opt[i+3];
>    123	
>    124				/* Never increase MSS, even when setting it, as
>    125				 * doing so results in problems for hosts that rely
>    126				 * on MSS being set correctly.
>    127				 */
>    128				if (oldmss <= newmss)
>    129					return 0;
>    130	
>    131				opt[i+2] = (newmss & 0xff00) >> 8;
>    132				opt[i+3] = newmss & 0x00ff;
>    133	
>    134				csum_oldmss = htons(oldmss);
>    135				csum_newmss = htons(newmss);
>    136	
>    137				/* MSS may be unaligned; fix up the incremental checksum
>    138				 * to avoid an invalid checksum and a dropped packet.
>    139				 */
>  > 140				if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {  
>    141					csum_oldmss = swab16(csum_oldmss);
>    142					csum_newmss = swab16(csum_newmss);
>    143				}
>    144	
>    145				inet_proto_csum_replace2(&tcph->check, skb,
>    146							 csum_oldmss, csum_newmss,
>    147							 false);
>    148				return 0;
>    149			}
>    150		}
>    151	
>    152		/* There is data after the header so the option can't be added
>    153		 * without moving it, and doing so may make the SYN packet
>    154		 * itself too large. Accept the packet unmodified instead.
>    155		 */
>    156		if (len > tcp_hdrlen)
>    157			return 0;
>    158	
>    159		/* tcph->doff has 4 bits, do not wrap it to 0 */
>    160		if (tcp_hdrlen >= 15 * 4)
>    161			return 0;
>    162	
>    163		/*
>    164		 * MSS Option not found ?! add it..
>    165		 */
>    166		if (skb_tailroom(skb) < TCPOLEN_MSS) {
>    167			if (pskb_expand_head(skb, 0,
>    168					     TCPOLEN_MSS - skb_tailroom(skb),
>    169					     GFP_ATOMIC))
>    170				return -1;
>    171			tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff);
>    172		}
>    173	
>    174		skb_put(skb, TCPOLEN_MSS);
>    175	
>    176		/*
>    177		 * IPv4: RFC 1122 states "If an MSS option is not received at
>    178		 * connection setup, TCP MUST assume a default send MSS of 536".
>    179		 * IPv6: RFC 2460 states IPv6 has a minimum MTU of 1280 and a minimum
>    180		 * length IPv6 header of 60, ergo the default MSS value is 1220
>    181		 * Since no MSS was provided, we must use the default values
>    182		 */
>    183		if (xt_family(par) == NFPROTO_IPV4)
>    184			newmss = min(newmss, (u16)536);
>    185		else
>    186			newmss = min(newmss, (u16)1220);
>    187	
>    188		opt = (u_int8_t *)tcph + sizeof(struct tcphdr);
>    189		memmove(opt + TCPOLEN_MSS, opt, len - sizeof(struct tcphdr));
>    190	
>    191		inet_proto_csum_replace2(&tcph->check, skb,
>    192					 htons(len), htons(len + TCPOLEN_MSS), true);
>    193		opt[0] = TCPOPT_MSS;
>    194		opt[1] = TCPOLEN_MSS;
>    195		opt[2] = (newmss & 0xff00) >> 8;
>    196		opt[3] = newmss & 0x00ff;
>    197	
>    198		inet_proto_csum_replace4(&tcph->check, skb, 0, *((__be32 *)opt), false);
>    199	
>    200		oldval = ((__be16 *)tcph)[6];
>    201		tcph->doff += TCPOLEN_MSS/4;
>    202		inet_proto_csum_replace2(&tcph->check, skb,
>    203					 oldval, ((__be16 *)tcph)[6], false);
>    204		return TCPOLEN_MSS;
>    205	}
>    206	
> 
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
  2026-05-25 20:11 [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned Kacper Kokot
  2026-05-25 21:28 ` Florian Westphal
  2026-05-26 13:50 ` kernel test robot
@ 2026-05-26 16:46 ` kernel test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2026-05-26 16:46 UTC (permalink / raw)
  To: Kacper Kokot, Pablo Neira Ayuso, Florian Westphal, Phil Sutter,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netfilter-devel, coreteam, linux-kernel
  Cc: llvm, oe-kbuild-all, netdev, stable, Kacper Kokot

Hi Kacper,

kernel test robot noticed the following build warnings:

[auto build test WARNING on nf-next/main]
[also build test WARNING on netfilter-nf/main linus/master v6.16-rc1 next-20260526]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kacper-Kokot/netfilter-TCPMSS-fix-dropped-packets-when-MSS-option-is-unaligned/20260526-041308
base:   https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git main
patch link:    https://lore.kernel.org/r/20260525201116.407338-2-kacper.kokot.44%40gmail.com
patch subject: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260526/202605261807.YY0PWuhX-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260526/202605261807.YY0PWuhX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605261807.YY0PWuhX-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/netfilter/xt_TCPMSS.c:140:45: warning: & has lower precedence than !=; != will be evaluated first [-Wparentheses]
     140 |                         if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {
         |                                                                  ^~~~~~~~~~
   net/netfilter/xt_TCPMSS.c:140:45: note: place parentheses around the '!=' expression to silence this warning
     140 |                         if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {
         |                                                                  ^         
         |                                                                    (       )
   net/netfilter/xt_TCPMSS.c:140:45: note: place parentheses around the & expression to evaluate it first
     140 |                         if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {
         |                                                                  ^
         |                             (                                         )
   1 warning generated.


vim +140 net/netfilter/xt_TCPMSS.c

    69	
    70	static int
    71	tcpmss_mangle_packet(struct sk_buff *skb,
    72			     const struct xt_action_param *par,
    73			     unsigned int family,
    74			     unsigned int tcphoff,
    75			     unsigned int minlen)
    76	{
    77		const struct xt_tcpmss_info *info = par->targinfo;
    78		struct tcphdr *tcph;
    79		int len, tcp_hdrlen;
    80		unsigned int i;
    81		__be16 oldval;
    82		u16 newmss;
    83		u8 *opt;
    84	
    85		/* This is a fragment, no TCP header is available */
    86		if (par->fragoff != 0)
    87			return 0;
    88	
    89		if (skb_ensure_writable(skb, skb->len))
    90			return -1;
    91	
    92		len = skb->len - tcphoff;
    93		if (len < (int)sizeof(struct tcphdr))
    94			return -1;
    95	
    96		tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff);
    97		tcp_hdrlen = tcph->doff * 4;
    98	
    99		if (len < tcp_hdrlen || tcp_hdrlen < sizeof(struct tcphdr))
   100			return -1;
   101	
   102		if (info->mss == XT_TCPMSS_CLAMP_PMTU) {
   103			struct net *net = xt_net(par);
   104			unsigned int in_mtu = tcpmss_reverse_mtu(net, skb, family);
   105			unsigned int min_mtu = min(dst_mtu(skb_dst(skb)), in_mtu);
   106	
   107			if (min_mtu <= minlen) {
   108				net_err_ratelimited("unknown or invalid path-MTU (%u)\n",
   109						    min_mtu);
   110				return -1;
   111			}
   112			newmss = min_mtu - minlen;
   113		} else
   114			newmss = info->mss;
   115	
   116		opt = (u_int8_t *)tcph;
   117		for (i = sizeof(struct tcphdr); i <= tcp_hdrlen - TCPOLEN_MSS; i += optlen(opt, i)) {
   118			if (opt[i] == TCPOPT_MSS && opt[i+1] == TCPOLEN_MSS) {
   119				u_int16_t oldmss;
   120				u16 csum_oldmss, csum_newmss;
   121	
   122				oldmss = (opt[i+2] << 8) | opt[i+3];
   123	
   124				/* Never increase MSS, even when setting it, as
   125				 * doing so results in problems for hosts that rely
   126				 * on MSS being set correctly.
   127				 */
   128				if (oldmss <= newmss)
   129					return 0;
   130	
   131				opt[i+2] = (newmss & 0xff00) >> 8;
   132				opt[i+3] = newmss & 0x00ff;
   133	
   134				csum_oldmss = htons(oldmss);
   135				csum_newmss = htons(newmss);
   136	
   137				/* MSS may be unaligned; fix up the incremental checksum
   138				 * to avoid an invalid checksum and a dropped packet.
   139				 */
 > 140				if (((char *)&opt[i + 2] - (char *)tcph) & 0x1 != 0) {
   141					csum_oldmss = swab16(csum_oldmss);
   142					csum_newmss = swab16(csum_newmss);
   143				}
   144	
   145				inet_proto_csum_replace2(&tcph->check, skb,
   146							 csum_oldmss, csum_newmss,
   147							 false);
   148				return 0;
   149			}
   150		}
   151	
   152		/* There is data after the header so the option can't be added
   153		 * without moving it, and doing so may make the SYN packet
   154		 * itself too large. Accept the packet unmodified instead.
   155		 */
   156		if (len > tcp_hdrlen)
   157			return 0;
   158	
   159		/* tcph->doff has 4 bits, do not wrap it to 0 */
   160		if (tcp_hdrlen >= 15 * 4)
   161			return 0;
   162	
   163		/*
   164		 * MSS Option not found ?! add it..
   165		 */
   166		if (skb_tailroom(skb) < TCPOLEN_MSS) {
   167			if (pskb_expand_head(skb, 0,
   168					     TCPOLEN_MSS - skb_tailroom(skb),
   169					     GFP_ATOMIC))
   170				return -1;
   171			tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff);
   172		}
   173	
   174		skb_put(skb, TCPOLEN_MSS);
   175	
   176		/*
   177		 * IPv4: RFC 1122 states "If an MSS option is not received at
   178		 * connection setup, TCP MUST assume a default send MSS of 536".
   179		 * IPv6: RFC 2460 states IPv6 has a minimum MTU of 1280 and a minimum
   180		 * length IPv6 header of 60, ergo the default MSS value is 1220
   181		 * Since no MSS was provided, we must use the default values
   182		 */
   183		if (xt_family(par) == NFPROTO_IPV4)
   184			newmss = min(newmss, (u16)536);
   185		else
   186			newmss = min(newmss, (u16)1220);
   187	
   188		opt = (u_int8_t *)tcph + sizeof(struct tcphdr);
   189		memmove(opt + TCPOLEN_MSS, opt, len - sizeof(struct tcphdr));
   190	
   191		inet_proto_csum_replace2(&tcph->check, skb,
   192					 htons(len), htons(len + TCPOLEN_MSS), true);
   193		opt[0] = TCPOPT_MSS;
   194		opt[1] = TCPOLEN_MSS;
   195		opt[2] = (newmss & 0xff00) >> 8;
   196		opt[3] = newmss & 0x00ff;
   197	
   198		inet_proto_csum_replace4(&tcph->check, skb, 0, *((__be32 *)opt), false);
   199	
   200		oldval = ((__be16 *)tcph)[6];
   201		tcph->doff += TCPOLEN_MSS/4;
   202		inet_proto_csum_replace2(&tcph->check, skb,
   203					 oldval, ((__be16 *)tcph)[6], false);
   204		return TCPOLEN_MSS;
   205	}
   206	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-26 16:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-25 20:11 [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned Kacper Kokot
2026-05-25 21:28 ` Florian Westphal
2026-05-25 21:44   ` Kacper Kokot
2026-05-25 22:08   ` Fernando Fernandez Mancera
2026-05-26  9:31     ` David Laight
2026-05-26 13:50 ` kernel test robot
2026-05-26 15:18   ` David Laight
2026-05-26 16:46 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox