lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <9a10520c-bfa9-2089-7d01-8ce215c48063@windriver.com> Date: Thu, 17 Aug 2023 11:32:43 +0800 From: heng guo <heng.guo@...driver.com> To: "David S. Miller" <davem@...emloft.net>, Alexey Kuznetsov <kuznet@....inr.ac.ru>, Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>, Jakub Kicinski <kuba@...nel.org> Cc: netdev@...r.kernel.org, "Guo, Heng" <Heng.Guo@...driver.com>, Filip Pudak <filip.pudak@...driver.com>, Richard.Danter@...driver.com Subject: [NETWORKING] IPSTATS_MIB_OUTOCTETS increment is duplicated in SNMP Hi maintainers, We find IPSTATS_MIB_OUTOCTETS increment is duplicated in SNMP test. *Test environment:* Linux version: 5.10.154 Platform: ARM nxp-ls1028 *Issue description:* As the attached network structure, SNMP is testing using tools scapy. Test command is "send(IP(dst="10.225.35.4",flags=0)/UDP()/Raw(load='0'*1400), count=1, inter=1.000000)" The test result shows that IPSTATS_MIB_OUTOCTETS increment is duplicated. *Debug:* Add dump_stack() in both ip_forward_finish() and ip_output(), also add debug logs in __SNMP_UPD_PO_STATS and __IP_ADD_STATS. Then get below test logs with core stack. ---------------------------------------------------------------------- kernel: ip_forward: NF_INET_FORWARD kernel: ip_forward_finish: increase IPSTATS_MIB_OUTOCTETS 1428 kernel: CPU: 0 PID: 12 Comm: ksoftirqd/0 Tainted: G W O 5.10.154-yocto-standard-wrl10.21.20.15 #1 kernel: Hardware name: freescale ls1028a/ls1028a, BIOS 2019.10+fsl+gd1aa7f1b3e kernel: Call trace: kernel: dump_backtrace+0x0/0x1a0 kernel: show_stack+0x20/0x30 kernel: dump_stack+0xcc/0x108 kernel: ip_forward_finish+0x178/0x1d4 kernel: ip_forward+0x59c/0x5fc kernel: ip_sublist_rcv_finish+0x50/0x70 kernel: ip_sublist_rcv+0x154/0x190 kernel: ip_list_rcv+0x100/0x1d0 kernel: __netif_receive_skb_list_core+0x188/0x21c kernel: netif_receive_skb_list_internal+0x1d8/0x2e4 kernel: napi_complete_done+0x70/0x1d4 kernel: gro_cell_poll+0x88/0xac kernel: net_rx_action+0x1b0/0x4fc kernel: __do_softirq+0x188/0x464 kernel: run_ksoftirqd+0x68/0x80 kernel: smpboot_thread_fn+0x204/0x240 kernel: kthread+0x14c/0x160 kernel: ret_from_fork+0x10/0x30 kernel: __IP_ADD_STATS, field:200, val:1428 kernel: __SNMP_ADD_STATS: mid:200, val:1428 kernel: __SNMP_ADD_STATS: mid:200, val:1428 kernel: ip_output: increase IPSTATS_MIB_OUT 1428 kernel: CPU: 0 PID: 12 Comm: ksoftirqd/0 Tainted: G W O 5.10.154-yocto-standard-wrl10.21.20.15 #1 kernel: Hardware name: freescale ls1028a/ls1028a, BIOS 2019.10+fsl+gd1aa7f1b3e kernel: Call trace: kernel: dump_backtrace+0x0/0x1a0 kernel: show_stack+0x20/0x30 kernel: dump_stack+0xcc/0x108 kernel: ip_output+0x2cc/0x2e4 kernel: ip_forward_finish+0x1b0/0x1d4 kernel: ip_forward+0x59c/0x5fc kernel: ip_sublist_rcv_finish+0x50/0x70 kernel: ip_sublist_rcv+0x154/0x190 kernel: ip_list_rcv+0x100/0x1d0 kernel: __netif_receive_skb_list_core+0x188/0x21c kernel: netif_receive_skb_list_internal+0x1d8/0x2e4 kernel: napi_complete_done+0x70/0x1d4 kernel: gro_cell_poll+0x88/0xac kernel: net_rx_action+0x1b0/0x4fc kernel: __do_softirq+0x188/0x464 kernel: run_ksoftirqd+0x68/0x80 kernel: smpboot_thread_fn+0x204/0x240 kernel: kthread+0x14c/0x160 kernel: ret_from_fork+0x10/0x30 kernel: SNMP_UPD_PO_STATS: mid:200, val:1428 kernel: SNMP_UPD_PO_STATS: mid:200, val:1428 kernel: SNMP_UPD_PO_STATS: mid:200, val:1428 kernel: net_dev_queue: { len:1428, name:tn_a.1073, network_header_type: IPV4, protocol: udp, saddr:[10.225.35.20], daddr:[10.225.35.4] } kernel: net_dev_queue: { len:1428, name:tn_a, network_header_type: IPV4, protocol: udp, saddr:[10.225.35.20], daddr:[10.225.35.4] } ----------------------------------------------------------------------- So the IPSTATS_MIB_OUTOCTETS is counted in both ip_forward_finish() and ip_output(). *Possible cause:* commit edf391ff1723 ("snmp: add missing counters for RFC 4293") had already added OutOctets for RFC 4293. In commit 2d8dbb04c63e ("snmp: fix OutOctets counter to include forwarded datagrams"), OutOctets was counted again, but not removed from ip_output(). And according to RFC 4293 "3.2.3. IP Statistics Tables"(Case-diagram.png), ipIfStatsOutTransmits is not equal to ipIfStatsOutForwDatagrams. So "IPSTATS_MIB_OUTOCTETS must be incremented when incrementing" is not accurate. And IPSTATS_MIB_OUTOCTETS should be counted after fragment. So do a fix patch for latest master branch, please help review it. Thanks, Heng *Patch:* ------------------------------------------------------ From ffe4b1107147562c5900fdc0c2c7dcd8ed9d1f37 Mon Sep 17 00:00:00 2001 From: Heng Guo <heng.guo@...driver.com> Date: Mon, 14 Aug 2023 14:49:47 +0800 Subject: [PATCH] SNMP: IPSTATS_MIB_OUTOCTETS increment duplicated commit edf391ff1723 ("snmp: add missing counters for RFC 4293") had already added OutOctets for RFC 4293. In commit 2d8dbb04c63e ("snmp: fix OutOctets counter to include forwarded datagrams"), OutOctets was counted again, but not removed from ip_output(). According to RFC 4293 "3.2.3. IP Statistics Tables", ipipIfStatsOutTransmits is not equal to ipIfStatsOutForwDatagrams. So "IPSTATS_MIB_OUTOCTETS must be incremented when incrementing" is not accurate. And IPSTATS_MIB_OUTOCTETS should be counted after fragment. This patch revert commit 2d8dbb04c63e and move IPSTATS_MIB_OUTOCTETS to ip_finish_output2 for ipv4. Reviewed-by: Filip Pudak <filip.pudak@...driver.com> Signed-off-by: Heng Guo <heng.guo@...driver.com> --- net/ipv4/ip_forward.c | 1 - net/ipv4/ip_output.c | 7 +++---- net/ipv4/ipmr.c | 1 - net/ipv6/ip6_output.c | 1 - net/ipv6/ip6mr.c | 2 -- 5 files changed, 3 insertions(+), 9 deletions(-) diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c index 29730edda220..0694f69ab25b 100644 --- a/net/ipv4/ip_forward.c +++ b/net/ipv4/ip_forward.c @@ -67,7 +67,6 @@ static int ip_forward_finish(struct net *net, struct sock *sk, struct sk_buff *s struct ip_options *opt = &(IPCB(skb)->opt); __IP_INC_STATS(net, IPSTATS_MIB_OUTFORWDATAGRAMS); - __IP_ADD_STATS(net, IPSTATS_MIB_OUTOCTETS, skb->len); #ifdef CONFIG_NET_SWITCHDEV if (skb->offload_l3_fwd_mark) { diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index ae8a456df5ab..f983d221eb92 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -205,6 +205,9 @@ static int ip_finish_output2(struct net *net, struct sock *sk, struct sk_buff *s } else if (rt->rt_type == RTN_BROADCAST) IP_UPD_PO_STATS(net, IPSTATS_MIB_OUTBCAST, skb->len); + /* OUTOCTETS should be counted after fragment */ + IP_UPD_PO_STATS(net, IPSTATS_MIB_OUT, skb->len); + if (unlikely(skb_headroom(skb) < hh_len && dev->header_ops)) { skb = skb_expand_head(skb, hh_len); if (!skb) @@ -364,8 +367,6 @@ int ip_mc_output(struct net *net, struct sock *sk, struct sk_buff *skb) /* * If the indicated interface is up and running, send the packet. */ - IP_UPD_PO_STATS(net, IPSTATS_MIB_OUT, skb->len); - skb->dev = dev; skb->protocol = htons(ETH_P_IP); @@ -422,8 +423,6 @@ int ip_output(struct net *net, struct sock *sk, struct sk_buff *skb) { struct net_device *dev = skb_dst(skb)->dev, *indev = skb->dev; - IP_UPD_PO_STATS(net, IPSTATS_MIB_OUT, skb->len); - skb->dev = dev; skb->protocol = htons(ETH_P_IP); diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c index aea29d97f8df..cdd626539f41 100644 --- a/net/ipv4/ipmr.c +++ b/net/ipv4/ipmr.c @@ -1780,7 +1780,6 @@ static inline int ipmr_forward_finish(struct net *net, struct sock *sk, struct ip_options *opt = &(IPCB(skb)->opt); IP_INC_STATS(net, IPSTATS_MIB_OUTFORWDATAGRAMS); - IP_ADD_STATS(net, IPSTATS_MIB_OUTOCTETS, skb->len); if (unlikely(opt->optlen)) ip_forward_options(skb); diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index be63929b1ac5..5ad8adeba151 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -431,7 +431,6 @@ static inline int ip6_forward_finish(struct net *net, struct sock *sk, struct dst_entry *dst = skb_dst(skb); __IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTFORWDATAGRAMS); - __IP6_ADD_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTOCTETS, skb->len); #ifdef CONFIG_NET_SWITCHDEV if (skb->offload_l3_fwd_mark) { diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c index 91f1c5f56d5f..c2c3180bde6a 100644 --- a/net/ipv6/ip6mr.c +++ b/net/ipv6/ip6mr.c @@ -1991,8 +1991,6 @@ static inline int ip6mr_forward2_finish(struct net *net, struct sock *sk, struct { IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_OUTFORWDATAGRAMS); - IP6_ADD_STATS(net, ip6_dst_idev(skb_dst(skb)), - IPSTATS_MIB_OUTOCTETS, skb->len); return dst_output(net, sk, skb); } -- 2.25.1 Content of type "text/html" skipped Download attachment "Case-diagram.png" of type "image/png" (77701 bytes) Download attachment "network-structure.png" of type "image/png" (49391 bytes)
Powered by blists - more mailing lists