* [PATCH] net: inet_diag: always export IPV6_V6ONLY sockopt
@ 2015-07-09 8:42 Phil Sutter
2015-07-09 12:57 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Phil Sutter @ 2015-07-09 8:42 UTC (permalink / raw)
To: netdev; +Cc: davem, Eric Dumazet
Reconsidering my commit 20462155 "net: inet_diag: export IPV6_V6ONLY
sockopt", I am not happy with the limitations it causes for socket
analysing code in userspace. Exporting the value only if it is set makes
it hard for userspace to decide whether the option is not set or the
kernel does not support exporting the option at all.
>From an auditor's perspective, the interesting question for AF_INET6
sockets is: "Does it NOT have IPV6_V6ONLY set?" Because it is the
unexpected case. This patch allows to answer this question reliably.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Cc: Eric Dumazet <edumazet@google.com>
---
net/ipv4/inet_diag.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 9bc2667..e622d2e 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -152,8 +152,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
inet6_sk(sk)->tclass) < 0)
goto errout;
- if (ipv6_only_sock(sk) &&
- nla_put_u8(skb, INET_DIAG_SKV6ONLY, 1))
+ if (nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
goto errout;
}
#endif
--
2.1.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: inet_diag: always export IPV6_V6ONLY sockopt
2015-07-09 8:42 [PATCH] net: inet_diag: always export IPV6_V6ONLY sockopt Phil Sutter
@ 2015-07-09 12:57 ` Eric Dumazet
2015-07-09 16:38 ` Phil Sutter
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2015-07-09 12:57 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev, davem, Eric Dumazet
On Thu, 2015-07-09 at 10:42 +0200, Phil Sutter wrote:
> Reconsidering my commit 20462155 "net: inet_diag: export IPV6_V6ONLY
> sockopt", I am not happy with the limitations it causes for socket
> analysing code in userspace. Exporting the value only if it is set makes
> it hard for userspace to decide whether the option is not set or the
> kernel does not support exporting the option at all.
>
> From an auditor's perspective, the interesting question for AF_INET6
> sockets is: "Does it NOT have IPV6_V6ONLY set?" Because it is the
> unexpected case. This patch allows to answer this question reliably.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> Cc: Eric Dumazet <edumazet@google.com>
> ---
> net/ipv4/inet_diag.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> index 9bc2667..e622d2e 100644
> --- a/net/ipv4/inet_diag.c
> +++ b/net/ipv4/inet_diag.c
> @@ -152,8 +152,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
> inet6_sk(sk)->tclass) < 0)
> goto errout;
>
> - if (ipv6_only_sock(sk) &&
> - nla_put_u8(skb, INET_DIAG_SKV6ONLY, 1))
> + if (nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
> goto errout;
> }
> #endif
Okay, I guess you don't dump millions of sockets.
Can you remind me why this attribute is useful for non listening
sockets ?
It looks you are interested to know if a _listener_ is v6 only or not.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: inet_diag: always export IPV6_V6ONLY sockopt
2015-07-09 12:57 ` Eric Dumazet
@ 2015-07-09 16:38 ` Phil Sutter
2015-07-09 17:30 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Phil Sutter @ 2015-07-09 16:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem
On Thu, Jul 09, 2015 at 02:57:57PM +0200, Eric Dumazet wrote:
> On Thu, 2015-07-09 at 10:42 +0200, Phil Sutter wrote:
> > Reconsidering my commit 20462155 "net: inet_diag: export IPV6_V6ONLY
> > sockopt", I am not happy with the limitations it causes for socket
> > analysing code in userspace. Exporting the value only if it is set makes
> > it hard for userspace to decide whether the option is not set or the
> > kernel does not support exporting the option at all.
> >
> > From an auditor's perspective, the interesting question for AF_INET6
> > sockets is: "Does it NOT have IPV6_V6ONLY set?" Because it is the
> > unexpected case. This patch allows to answer this question reliably.
> >
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > Cc: Eric Dumazet <edumazet@google.com>
> > ---
> > net/ipv4/inet_diag.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> > index 9bc2667..e622d2e 100644
> > --- a/net/ipv4/inet_diag.c
> > +++ b/net/ipv4/inet_diag.c
> > @@ -152,8 +152,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
> > inet6_sk(sk)->tclass) < 0)
> > goto errout;
> >
> > - if (ipv6_only_sock(sk) &&
> > - nla_put_u8(skb, INET_DIAG_SKV6ONLY, 1))
> > + if (nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
> > goto errout;
> > }
> > #endif
>
> Okay, I guess you don't dump millions of sockets.
I hope not, although I don't know what tools other than ss use
inet_diag.
> Can you remind me why this attribute is useful for non listening
> sockets ?
Indeed, it isn't.
> It looks you are interested to know if a _listener_ is v6 only or not.
Is it possible to select that in a generic way? If I didn't misread the
code, 'ss -l' filters them out locally. It looks like sk->sk_state can
be used to determine that, but it seems to be used for TCP states only?
Or is the value 10 (TCP_LISTEN) used for e.g. UDP as well?
Thanks, Phil
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: inet_diag: always export IPV6_V6ONLY sockopt
2015-07-09 16:38 ` Phil Sutter
@ 2015-07-09 17:30 ` Eric Dumazet
2015-07-10 9:39 ` [PATCH v2] net: inet_diag: always export IPV6_V6ONLY sockopt for listening sockets Phil Sutter
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2015-07-09 17:30 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev, davem
On Thu, 2015-07-09 at 18:38 +0200, Phil Sutter wrote:
> Is it possible to select that in a generic way? If I didn't misread the
> code, 'ss -l' filters them out locally. It looks like sk->sk_state can
> be used to determine that, but it seems to be used for TCP states only?
> Or is the value 10 (TCP_LISTEN) used for e.g. UDP as well?
You could specifically filter TCP_LISTEN | TCP_CLOSE
Like in :
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 9bc26677058e..c3b1f3a0f4cf 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -152,8 +152,8 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
inet6_sk(sk)->tclass) < 0)
goto errout;
- if (ipv6_only_sock(sk) &&
- nla_put_u8(skb, INET_DIAG_SKV6ONLY, 1))
+ if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
+ nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
goto errout;
}
#endif
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] net: inet_diag: always export IPV6_V6ONLY sockopt for listening sockets
2015-07-09 17:30 ` Eric Dumazet
@ 2015-07-10 9:39 ` Phil Sutter
2015-07-11 6:25 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Phil Sutter @ 2015-07-10 9:39 UTC (permalink / raw)
To: netdev; +Cc: davem, Eric Dumazet
Reconsidering my commit 20462155 "net: inet_diag: export IPV6_V6ONLY
sockopt", I am not happy with the limitations it causes for socket
analysing code in userspace. Exporting the value only if it is set makes
it hard for userspace to decide whether the option is not set or the
kernel does not support exporting the option at all.
>From an auditor's perspective, the interesting question for listening
AF_INET6 sockets is: "Does it NOT have IPV6_V6ONLY set?" Because it is
the unexpected case. This patch allows to answer this question reliably.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Cc: Eric Dumazet <edumazet@google.com>
---
Changes since v1:
- Export the value only for listening sockets, as suggested by Eric Dumazet.
- Adjusted commit message accordingly.
---
net/ipv4/inet_diag.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 9bc2667..c3b1f3a 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -152,8 +152,8 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
inet6_sk(sk)->tclass) < 0)
goto errout;
- if (ipv6_only_sock(sk) &&
- nla_put_u8(skb, INET_DIAG_SKV6ONLY, 1))
+ if (((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
+ nla_put_u8(skb, INET_DIAG_SKV6ONLY, ipv6_only_sock(sk)))
goto errout;
}
#endif
--
2.1.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] net: inet_diag: always export IPV6_V6ONLY sockopt for listening sockets
2015-07-10 9:39 ` [PATCH v2] net: inet_diag: always export IPV6_V6ONLY sockopt for listening sockets Phil Sutter
@ 2015-07-11 6:25 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2015-07-11 6:25 UTC (permalink / raw)
To: phil; +Cc: netdev, edumazet
From: Phil Sutter <phil@nwl.cc>
Date: Fri, 10 Jul 2015 11:39:57 +0200
> Reconsidering my commit 20462155 "net: inet_diag: export IPV6_V6ONLY
> sockopt", I am not happy with the limitations it causes for socket
> analysing code in userspace. Exporting the value only if it is set makes
> it hard for userspace to decide whether the option is not set or the
> kernel does not support exporting the option at all.
>
> From an auditor's perspective, the interesting question for listening
> AF_INET6 sockets is: "Does it NOT have IPV6_V6ONLY set?" Because it is
> the unexpected case. This patch allows to answer this question reliably.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> Cc: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-11 6:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-09 8:42 [PATCH] net: inet_diag: always export IPV6_V6ONLY sockopt Phil Sutter
2015-07-09 12:57 ` Eric Dumazet
2015-07-09 16:38 ` Phil Sutter
2015-07-09 17:30 ` Eric Dumazet
2015-07-10 9:39 ` [PATCH v2] net: inet_diag: always export IPV6_V6ONLY sockopt for listening sockets Phil Sutter
2015-07-11 6:25 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).