[PATCH net] vxlan: Do not reuse sockets for a different address family

[PATCH net] vxlan: Do not reuse sockets for a different address family

[Marcelo Ricardo Leitner]

Currently, we only match against local port number in order to reuse
socket. But if this new vxlan wants an IPv6 socket and a IPv4 one bound
to that port, vxlan will reuse an IPv4 socket as IPv6 and a panic will
follow. The following steps reproduce it:

   # ip link add vxlan6 type vxlan id 42 group 229.10.10.10 \
       srcport 5000 6000 dev eth0
   # ip link add vxlan7 type vxlan id 43 group ff0e::110 \
       srcport 5000 6000 dev eth0
   # ip link set vxlan6 up
   # ip link set vxlan7 up
   <panic>

[    4.187481] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058
[    4.187509] IP: [<ffffffff81667c98>] ipv6_sock_mc_join+0x88/0x630
...
[    4.188076] Call Trace:
[    4.188085]  [<ffffffff81667c4a>] ? ipv6_sock_mc_join+0x3a/0x630
[    4.188098]  [<ffffffffa05a6ad6>] vxlan_igmp_join+0x66/0xd0 [vxlan]
[    4.188113]  [<ffffffff810a3430>] process_one_work+0x220/0x710
[    4.188125]  [<ffffffff810a33c4>] ? process_one_work+0x1b4/0x710
[    4.188138]  [<ffffffff810a3a3b>] worker_thread+0x11b/0x3a0
[    4.188149]  [<ffffffff810a3920>] ? process_one_work+0x710/0x710

So address family must also match in order to reuse a socket.

Reported-by: Jean-Tsung Hsiao <jhsiao@redhat.com>
Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
---
 drivers/net/vxlan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index ca309820d39e1ba7995f38d3a2f9bacbd1c1f857..c0fa76d55ae3cc07fb14b70656d6b13b5bab091c 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -281,7 +281,8 @@ static struct vxlan_sock *vxlan_find_sock(struct net *net, __be16 port)
 	struct vxlan_sock *vs;
 
 	hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
-		if (inet_sk(vs->sock->sk)->inet_sport == port)
+		if ((inet_sk(vs->sock->sk)->inet_sport == port) &&
+			(inet_sk(vs->sock->sk)->sk.sk_family == family))
 			return vs;
 	}
 	return NULL;
-- 
1.9.3

Re: [PATCH net] vxlan: Do not reuse sockets for a different address family

[Sergei Shtylyov]

Hello.

On 11/4/2014 4:46 PM, Marcelo Ricardo Leitner wrote:

> Currently, we only match against local port number in order to reuse
> socket. But if this new vxlan wants an IPv6 socket and a IPv4 one bound
> to that port, vxlan will reuse an IPv4 socket as IPv6 and a panic will
> follow. The following steps reproduce it:

>     # ip link add vxlan6 type vxlan id 42 group 229.10.10.10 \
>         srcport 5000 6000 dev eth0
>     # ip link add vxlan7 type vxlan id 43 group ff0e::110 \
>         srcport 5000 6000 dev eth0
>     # ip link set vxlan6 up
>     # ip link set vxlan7 up
>     <panic>

> [    4.187481] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058
> [    4.187509] IP: [<ffffffff81667c98>] ipv6_sock_mc_join+0x88/0x630
> ...
> [    4.188076] Call Trace:
> [    4.188085]  [<ffffffff81667c4a>] ? ipv6_sock_mc_join+0x3a/0x630
> [    4.188098]  [<ffffffffa05a6ad6>] vxlan_igmp_join+0x66/0xd0 [vxlan]
> [    4.188113]  [<ffffffff810a3430>] process_one_work+0x220/0x710
> [    4.188125]  [<ffffffff810a33c4>] ? process_one_work+0x1b4/0x710
> [    4.188138]  [<ffffffff810a3a3b>] worker_thread+0x11b/0x3a0
> [    4.188149]  [<ffffffff810a3920>] ? process_one_work+0x710/0x710

> So address family must also match in order to reuse a socket.

> Reported-by: Jean-Tsung Hsiao <jhsiao@redhat.com>
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> ---
>   drivers/net/vxlan.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index ca309820d39e1ba7995f38d3a2f9bacbd1c1f857..c0fa76d55ae3cc07fb14b70656d6b13b5bab091c 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -281,7 +281,8 @@ static struct vxlan_sock *vxlan_find_sock(struct net *net, __be16 port)
>   	struct vxlan_sock *vs;
>
>   	hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
> -		if (inet_sk(vs->sock->sk)->inet_sport == port)
> +		if ((inet_sk(vs->sock->sk)->inet_sport == port) &&
> +			(inet_sk(vs->sock->sk)->sk.sk_family == family))

   The continuation line should start below the next character after ( of the 
*if* statement, according to the networking coding style. And inner () are not 
necessary.

WBR, Sergei

Re: [PATCH net] vxlan: Do not reuse sockets for a different address family

[David Miller]

From: Marcelo Ricardo Leitner <mleitner@redhat.com>
Date: Tue,  4 Nov 2014 11:46:49 -0200

>  	hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
> -		if (inet_sk(vs->sock->sk)->inet_sport == port)
> +		if ((inet_sk(vs->sock->sk)->inet_sport == port) &&
> +			(inet_sk(vs->sock->sk)->sk.sk_family == family))

This is not indented properly.

For a multi-line conditional, the second and subsequent lines
should start precisely at the first column after the openning
parenthesis on the first line.

You must use the appropriate number of TAB and SPACE characters
necessary to achieve this.  If you are only using TAB characters, you
are most likely doing it wrong.

Please fix this up and resubmit, thanks.

Re: [PATCH net] vxlan: Do not reuse sockets for a different address family

[Marcelo Ricardo Leitner]

On 04-11-2014 14:32, David Miller wrote:
> From: Marcelo Ricardo Leitner <mleitner@redhat.com>
> Date: Tue,  4 Nov 2014 11:46:49 -0200
>
>>   	hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
>> -		if (inet_sk(vs->sock->sk)->inet_sport == port)
>> +		if ((inet_sk(vs->sock->sk)->inet_sport == port) &&
>> +			(inet_sk(vs->sock->sk)->sk.sk_family == family))
>
> This is not indented properly.
>
> For a multi-line conditional, the second and subsequent lines
> should start precisely at the first column after the openning
> parenthesis on the first line.
>
> You must use the appropriate number of TAB and SPACE characters
> necessary to achieve this.  If you are only using TAB characters, you
> are most likely doing it wrong.

Yes.. I forgot to switch profiles, was using tab spacing=4, then it was 
aligned and I didn't notice it.

> Please fix this up and resubmit, thanks.

Okay, sorry. Thanks David, Sergei.

Marcelo