Building the Linux kernel with Clang and LLVM
 help / color / mirror / Atom feed
* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
       [not found] <20260525201116.407338-2-kacper.kokot.44@gmail.com>
@ 2026-05-26 16:46 ` kernel test robot
  2026-05-26 23:21   ` Kacper Kokot
  2026-05-28 18:11 ` kernel test robot
  1 sibling, 1 reply; 4+ 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] 4+ messages in thread

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

> 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 agree, I haven't observed it in any real environment and wouldn't expect to.
I found it by reading the code and had to craft a SYN to reproduce. That said
the spec permits unaligned options and the kernel shouldn't silently drop legal
packets just because nobody sends them today. I can note in the v2 commit
message that this is a theoretical fix.

> > 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.

Happy to go with whichever you prefer for v2.

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

Ha - accidentally correct. I'll add the parens in v2 tomorrow.

Also the reproducer I sent with v1 was clunky. Here's a better
one with some results below:

  #!/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 gen_mss_syn_options(nops=0):
      return nops * [("NOP", None)] + [("MSS", 1460)]

  def syn_check(opts):
      sport = RandShort()
      ip = IP(dst=args.target_ip)
      syn = TCP(sport=sport, dport=args.target_port, flags="S",
seq=1000, options=opts)
      synack = sr1(ip/syn, timeout=1, verbose=False)
      send(ip/TCP(sport=sport, dport=args.target_port, flags="R",
seq=syn.seq+1),
           verbose=False)
      return not not (synack and synack.haslayer(TCP) and
synack[TCP].flags == 0x12)

  for i in range(7):
      n = 5
      ok = sum(syn_check(gen_mss_syn_options(i)) for _ in range(n))
      print(f"{i} nops + mss, {ok}/{n} probes responded")

Before:

  0 nops + mss, 5/5 probes responded
  1 nops + mss, 0/5 probes responded
  2 nops + mss, 5/5 probes responded
  3 nops + mss, 0/5 probes responded
  4 nops + mss, 5/5 probes responded
  5 nops + mss, 0/5 probes responded
  6 nops + mss, 5/5 probes responded

After:

  0 nops + mss, 5/5 probes responded
  1 nops + mss, 5/5 probes responded
  2 nops + mss, 5/5 probes responded
  3 nops + mss, 5/5 probes responded
  4 nops + mss, 5/5 probes responded
  5 nops + mss, 5/5 probes responded
  6 nops + mss, 5/5 probes responded

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

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
       [not found] <20260525201116.407338-2-kacper.kokot.44@gmail.com>
  2026-05-26 16:46 ` [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned kernel test robot
@ 2026-05-28 18:11 ` kernel test robot
  2026-05-28 19:40   ` David Laight
  1 sibling, 1 reply; 4+ messages in thread
From: kernel test robot @ 2026-05-28 18:11 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 v7.1-rc5 next-20260527]
[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: s390-allmodconfig (https://download.01.org/0day-ci/archive/20260529/202605290221.PE1wkPWQ-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260529/202605290221.PE1wkPWQ-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/202605290221.PE1wkPWQ-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] 4+ messages in thread

* Re: [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned
  2026-05-28 18:11 ` kernel test robot
@ 2026-05-28 19:40   ` David Laight
  0 siblings, 0 replies; 4+ messages in thread
From: David Laight @ 2026-05-28 19:40 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, llvm,
	oe-kbuild-all, netdev, stable

On Fri, 29 May 2026 02:11:48 +0800
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 v7.1-rc5 next-20260527]
> [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: s390-allmodconfig (https://download.01.org/0day-ci/archive/20260529/202605290221.PE1wkPWQ-lkp@intel.com/config)
> compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260529/202605290221.PE1wkPWQ-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/202605290221.PE1wkPWQ-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.

This has been noted a lot of times.
While 'unusual' the expression is actually correct (but does need changing).
Mostly because the '!= 0' isn't needed and '0x1 != 0' is 1.

-- 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] 4+ messages in thread

end of thread, other threads:[~2026-05-28 19:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260525201116.407338-2-kacper.kokot.44@gmail.com>
2026-05-26 16:46 ` [PATCH] netfilter: TCPMSS: fix dropped packets when MSS option is unaligned kernel test robot
2026-05-26 23:21   ` Kacper Kokot
2026-05-28 18:11 ` kernel test robot
2026-05-28 19:40   ` David Laight

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