| 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