Re: [net-ipv4] question about arguments position

From: Gustavo A. R. Silva
Date: Thu May 04 2017 - 15:45:25 EST

Hi Joe,

Quoting Joe Perches <joe@xxxxxxxxxxx>:

On Thu, 2017-05-04 at 12:46 -0400, David Miller wrote:
From: "Gustavo A. R. Silva" <garsilva@xxxxxxxxxxxxxx>
Date: Thu, 04 May 2017 11:07:54 -0500

> While looking into Coverity ID 1357474 I ran into the following piece
> of code at net/ipv4/inet_diag.c:392:

Because it's been this way since at least 2005, it doesn't matter if
the order is correct or not. What's there is the locked in behavior
exposed to userspace and changing it will break things for people.

Adding a few comments around the code about why
it is this way will help avoid future questions.

In the case of Coverity, I already triaged and documented this issue. So people can ignore it in the future.

Regarding the code comments, what about the following patch:

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 3828b3a..7a56641 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -389,6 +389,12 @@ static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
nlmsg_flags, unlh, net_admin);

+ * Ignore the position of the arguments req->id.idiag_dport and
+ * req->id.idiag_sport in both calls to inet_lookup() and inet6_lookup()
+ * functions, once this is a locked in behavior exposed to user space.
+ * Changing this will break things for people.
+ */
struct sock *inet_diag_find_one_icsk(struct net *net,
struct inet_hashinfo *hashinfo,
const struct inet_diag_req_v2 *req)

Gustavo A. R. Silva