netdev - [PATCH iproute2-next] ip: ipstats: Do not assume length of response attribute payload

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>] [day] [month] [year] [list]

Message-ID: <354708bc6198a082432f1119ab4336fdaf5e5342.1651764420.git.petrm@nvidia.com>
Date:   Thu, 5 May 2022 17:31:34 +0200
From:   Petr Machata <petrm@...dia.com>
To:     <netdev@...r.kernel.org>
CC:     David Ahern <dsahern@...il.com>, Ido Schimmel <idosch@...dia.com>,
        "Petr Machata" <petrm@...dia.com>
Subject: [PATCH iproute2-next] ip: ipstats: Do not assume length of response attribute payload

In Linux kernel commit 794c24e9921f ("net-core: rx_otherhost_dropped to
core_stats"), struct rtnl_link_stats64 got a new member. This change got to
iproute2 through commit bba95837524d ("Update kernel headers").

"ip stats" makes the assumption that the payload of attributes that carry
structures is at least as long as the size of the given structure as
iproute2 knows it. But that will not hold when a newer iproute2 is used
against an older kernel: since such kernel misses some fields on the tail
end of the structure, "ip stats" bails out:

 # ip stats show group link
 1: lo: group link
 Error: attribute payload too shortDump terminated

Instead, be tolerant of responses that are both longer and shorter than
what is expected. Instead of forming a pointer directly into the payload,
allocate the stats structure on the stack, zero it, and then copy over the
portion from the response.

Reported-by: Ido Schimmel <idosch@...dia.com>
Signed-off-by: Petr Machata <petrm@...dia.com>
Reviewed-by: Ido Schimmel <idosch@...dia.com>
Tested-by: Ido Schimmel <idosch@...dia.com>
---
 ip/ipstats.c | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/ip/ipstats.c b/ip/ipstats.c
index 39ddca01..5b9689f4 100644
--- a/ip/ipstats.c
+++ b/ip/ipstats.c
@@ -130,21 +130,21 @@ ipstats_stat_show_attrs_free(struct ipstats_stat_show_attrs *attrs)
 		free(attrs->tbs[i]);
 }
 
-#define IPSTATS_RTA_PAYLOAD(TYPE, AT)					\
-	({								\
+#define IPSTATS_RTA_PAYLOAD(VAR, AT)					\
+	do {								\
 		const struct rtattr *__at = (AT);			\
-		TYPE *__ret = NULL;					\
+		size_t __at_sz = __at->rta_len - RTA_LENGTH(0);		\
+		size_t __var_sz = sizeof(VAR);				\
+		typeof(VAR) *__dest = &VAR;				\
 									\
-		if (__at != NULL &&					\
-		    __at->rta_len - RTA_LENGTH(0) >= sizeof(TYPE))	\
-			__ret = RTA_DATA(__at);				\
-		__ret;							\
-	})
+		memset(__dest, 0, __var_sz);				\
+		memcpy(__dest, RTA_DATA(__at), MIN(__at_sz, __var_sz));	\
+	} while (0)
 
 static int ipstats_show_64(struct ipstats_stat_show_attrs *attrs,
 			   unsigned int group, unsigned int subgroup)
 {
-	struct rtnl_link_stats64 *stats;
+	struct rtnl_link_stats64 stats;
 	const struct rtattr *at;
 	int err;
 
@@ -152,14 +152,10 @@ static int ipstats_show_64(struct ipstats_stat_show_attrs *attrs,
 	if (at == NULL)
 		return err;
 
-	stats = IPSTATS_RTA_PAYLOAD(struct rtnl_link_stats64, at);
-	if (stats == NULL) {
-		fprintf(stderr, "Error: attribute payload too short");
-		return -EINVAL;
-	}
+	IPSTATS_RTA_PAYLOAD(stats, at);
 
 	open_json_object("stats64");
-	print_stats64(stdout, stats, NULL, NULL);
+	print_stats64(stdout, &stats, NULL, NULL);
 	close_json_object();
 	return 0;
 }
@@ -228,15 +224,10 @@ static void print_hw_stats64(FILE *fp, struct rtnl_hw_stats64 *s)
 
 static int ipstats_show_hw64(const struct rtattr *at)
 {
-	struct rtnl_hw_stats64 *stats;
+	struct rtnl_hw_stats64 stats;
 
-	stats = IPSTATS_RTA_PAYLOAD(struct rtnl_hw_stats64, at);
-	if (stats == NULL) {
-		fprintf(stderr, "Error: attribute payload too short");
-		return -EINVAL;
-	}
-
-	print_hw_stats64(stdout, stats);
+	IPSTATS_RTA_PAYLOAD(stats, at);
+	print_hw_stats64(stdout, &stats);
 	return 0;
 }
 
-- 
2.31.1