netdev - [PATCH net] net: diag: Fix swapped src/dst in udp_dump

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

Message-Id: <20180914062553.42648-1-lorenzo@google.com>
Date:   Fri, 14 Sep 2018 15:25:53 +0900
From:   Lorenzo Colitti <lorenzo@...gle.com>
To:     netdev@...r.kernel.org
Cc:     davem@...emloft.net, zenczykowski@...il.com, dsahern@...il.com,
        jeffv@...gle.com, Lorenzo Colitti <lorenzo@...gle.com>
Subject: [PATCH net] net: diag: Fix swapped src/dst in udp_dump_one.

Since its inception, udp_dump_one had has a bug where userspace
needs to swap src and dst addresses and ports in order to find
the socket it wants.

This is because udp_dump_one misuses __udp[46]_lib_lookup by
passing the source address as the source address argument.
Unfortunately, those functions are intended to find local sockets
matching received packets, so the order of the arguments is
inverted: the argument that ends up being compared with, e.g.,
sk_daddr is actually saddr, not daddr.

While it's true that this creates a backwards compatibility
problem, this is clearly a bug since inet_diag_sockid is very
clear about which struct elements are the source address and port
and which are the destination address and port. Also, this bug
does not affect TCP sockets, SOCK_DESTROY of UDP sockets, or
finding UDP sockets with NLMSG_DUMP.

Fixes: a925aa00a55 ("udp_diag: Implement the get_exact dumping functionality")
Tested: https://android-review.googlesource.com/c/kernel/tests/+/755889/
Signed-off-by: Lorenzo Colitti <lorenzo@...gle.com>
---
 net/ipv4/udp_diag.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/udp_diag.c b/net/ipv4/udp_diag.c
index d9ad986c7b..e1c6f90a92 100644
--- a/net/ipv4/udp_diag.c
+++ b/net/ipv4/udp_diag.c
@@ -43,16 +43,16 @@ static int udp_dump_one(struct udp_table *tbl, struct sk_buff *in_skb,
 	rcu_read_lock();
 	if (req->sdiag_family == AF_INET)
 		sk = __udp4_lib_lookup(net,
-				req->id.idiag_src[0], req->id.idiag_sport,
 				req->id.idiag_dst[0], req->id.idiag_dport,
+				req->id.idiag_src[0], req->id.idiag_sport,
 				req->id.idiag_if, 0, tbl, NULL);
 #if IS_ENABLED(CONFIG_IPV6)
 	else if (req->sdiag_family == AF_INET6)
 		sk = __udp6_lib_lookup(net,
-				(struct in6_addr *)req->id.idiag_src,
-				req->id.idiag_sport,
 				(struct in6_addr *)req->id.idiag_dst,
 				req->id.idiag_dport,
+				(struct in6_addr *)req->id.idiag_src,
+				req->id.idiag_sport,
 				req->id.idiag_if, 0, tbl, NULL);
 #endif
 	if (sk && !refcount_inc_not_zero(&sk->sk_refcnt))
-- 
2.19.0.397.gdd90340f6a-goog