netdev - [PATCH iproute2 v2] ip-monitor: Do not error out when RTNLGRP

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20221026064907.1209952-1-bpoirier@nvidia.com>
Date:   Wed, 26 Oct 2022 15:49:07 +0900
From:   Benjamin Poirier <bpoirier@...dia.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org, David Ahern <dsahern@...nel.org>,
        Ido Schimmel <idosch@...dia.com>
Subject: [PATCH iproute2 v2] ip-monitor: Do not error out when RTNLGRP_STATS is not available

Following commit 4e8a9914c4d4 ("ip-monitor: Include stats events in default
and "all" cases"), `ip monitor` fails to start on kernels which do not
contain linux.git commit 5fd0b838efac ("net: rtnetlink: Add UAPI toggle for
IFLA_OFFLOAD_XSTATS_L3_STATS") because the netlink group RTNLGRP_STATS
doesn't exist:

 $ ip monitor
 Failed to add stats group to list

When "stats" is not explicitly requested, ignore the error so that `ip
monitor` and `ip monitor all` continue to work on older kernels.

Note that the same change is not done for RTNLGRP_NEXTHOP because its value
is 32 and group numbers <= 32 are always supported; see the comment above
netlink_change_ngroups() in the kernel source. Therefore
NETLINK_ADD_MEMBERSHIP 32 does not error out even on kernels which do not
support RTNLGRP_NEXTHOP.

v2:
* Silently ignore a failure to implicitly add the stats group, instead of
  printing a warning.

Reported-by: Stephen Hemminger <stephen@...workplumber.org>
Fixes: 4e8a9914c4d4 ("ip-monitor: Include stats events in default and "all" cases")
Signed-off-by: Benjamin Poirier <bpoirier@...dia.com>
---

> There are two acceptable solutions:
> 1. Ignore the error, and never print any warning.
> 2. Don't ask for the stats feature with the default "ip monitor" and "ip monitor all"
> 
> Either way, it needs to be totally silent when built and run on older kernels.

Strictly speaking, the patch below is solution 1*:
  Ignore the error, and never print any warning ... when implicitly
adding the stats group.

Before 4e8a9914c4d4, `ip mon stats` used to error out if the stats group
could not be added. That behavior is preserved.


 ip/ipmonitor.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 8a72ea42..d808369c 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -195,6 +195,8 @@ static int accept_msg(struct rtnl_ctrl_data *ctrl,
 int do_ipmonitor(int argc, char **argv)
 {
 	unsigned int groups = 0, lmask = 0;
+	/* "needed" mask, failure to enable is an error */
+	unsigned int nmask;
 	char *file = NULL;
 	int ifindex = 0;
 
@@ -253,6 +255,7 @@ int do_ipmonitor(int argc, char **argv)
 	ipneigh_reset_filter(ifindex);
 	ipnetconf_reset_filter(ifindex);
 
+	nmask = lmask;
 	if (!lmask)
 		lmask = IPMON_L_ALL;
 
@@ -327,7 +330,8 @@ int do_ipmonitor(int argc, char **argv)
 	}
 
 	if (lmask & IPMON_LSTATS &&
-	    rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0) {
+	    rtnl_add_nl_group(&rth, RTNLGRP_STATS) < 0 &&
+	    nmask & IPMON_LSTATS) {
 		fprintf(stderr, "Failed to add stats group to list\n");
 		exit(1);
 	}
-- 
2.37.2