netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] arp: filter NOARP neighbours for SIOCGARP
@ 2015-07-27  9:33 Eric Dumazet
  2015-07-29  6:41 ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2015-07-27  9:33 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Vytautas Valancius, Willem de Bruijn

From: Eric Dumazet <edumazet@google.com>

When arp is off on a device, and ioctl(SIOCGARP) is queried,
a buggy answer is given with MAC address of the device, instead
of the mac address of the destination/gateway.

We filter out NUD_NOARP neighbours for /proc/net/arp,
we must do the same for SIOCGARP ioctl.

Tested:

lpaa23:~# ./arp 10.246.7.190
MAC=00:01:e8:22:cb:1d      // correct answer

lpaa23:~# ip link set dev eth0 arp off
lpaa23:~# cat /proc/net/arp   # check arp table is now 'empty'
IP address       HW type     Flags       HW address    Mask     Device
lpaa23:~# ./arp 10.246.7.190
MAC=00:1a:11:c3:0d:7f   // buggy answer before patch (this is eth0 mac)

After patch :

lpaa23:~# ip link set dev eth0 arp off
lpaa23:~# ./arp 10.246.7.190
ioctl(SIOCGARP) failed: No such device or address

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Vytautas Valancius <valas@google.com>
Cc: Willem de Bruijn <willemb@google.com>
---
 net/ipv4/arp.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 933a92820d26..6c8b1fbafce8 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1017,14 +1017,16 @@ static int arp_req_get(struct arpreq *r, struct net_device *dev)
 
 	neigh = neigh_lookup(&arp_tbl, &ip, dev);
 	if (neigh) {
-		read_lock_bh(&neigh->lock);
-		memcpy(r->arp_ha.sa_data, neigh->ha, dev->addr_len);
-		r->arp_flags = arp_state_to_flags(neigh);
-		read_unlock_bh(&neigh->lock);
-		r->arp_ha.sa_family = dev->type;
-		strlcpy(r->arp_dev, dev->name, sizeof(r->arp_dev));
+		if (!(neigh->nud_state & NUD_NOARP)) {
+			read_lock_bh(&neigh->lock);
+			memcpy(r->arp_ha.sa_data, neigh->ha, dev->addr_len);
+			r->arp_flags = arp_state_to_flags(neigh);
+			read_unlock_bh(&neigh->lock);
+			r->arp_ha.sa_family = dev->type;
+			strlcpy(r->arp_dev, dev->name, sizeof(r->arp_dev));
+			err = 0;
+		}
 		neigh_release(neigh);
-		err = 0;
 	}
 	return err;
 }

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net] arp: filter NOARP neighbours for SIOCGARP
  2015-07-27  9:33 [PATCH net] arp: filter NOARP neighbours for SIOCGARP Eric Dumazet
@ 2015-07-29  6:41 ` David Miller
  2015-07-29  9:15   ` Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2015-07-29  6:41 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, valas, willemb

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 27 Jul 2015 11:33:50 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> When arp is off on a device, and ioctl(SIOCGARP) is queried,
> a buggy answer is given with MAC address of the device, instead
> of the mac address of the destination/gateway.
> 
> We filter out NUD_NOARP neighbours for /proc/net/arp,
> we must do the same for SIOCGARP ioctl.
> 
> Tested:
 ...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Vytautas Valancius <valas@google.com>

Applied, thanks Eric.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] arp: filter NOARP neighbours for SIOCGARP
  2015-07-29  6:41 ` David Miller
@ 2015-07-29  9:15   ` Eric Dumazet
  2015-07-29  9:32     ` Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2015-07-29  9:15 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, valas, willemb

On Tue, 2015-07-28 at 23:41 -0700, David Miller wrote:

> Applied, thanks Eric.
Thanks David

Note that IPv6 is completely broken after 'arp off' sequence.

ND destination MAC are a copy of eth0 MAC address, instead of the required multicast.

lpaa23:~# ip link set dev eth0 arp off
lpaa23:~# ping6 4444::555:0027
PING 4444::555:0027(4444::555:27) 56 data bytes
02:05:13.742684 00:1a:11:c3:0d:7f > 00:1a:11:c3:0d:7f, ethertype IPv6 (0x86dd), length 118: 4444::555:23 > 4444::555:27: ICMP6, echo request, seq 1, length 64
02:05:14.742200 00:1a:11:c3:0d:7f > 00:1a:11:c3:0d:7f, ethertype IPv6 (0x86dd), length 118: 4444::555:23 > 4444::555:27: ICMP6, echo request, seq 2, length 64
^C
--- 4444::555:0027 ping statistics ---
2 packets transmitted, 0 received, 100% packet loss, time 999ms

lpaa23:~# ip link set dev eth0 arp on 

lpaa23:~# ip -6 neigh sh dev eth0
fe80::21a:11ff:fec3:d45 lladdr 00:1a:11:c3:0d:45 STALE
4444::555:26  FAILED
4444::555:25  FAILED

lpaa23:~# ping6 4444::555:0027
PING 4444::555:0027(4444::555:27) 56 data bytes
02:12:15.698654 00:1a:11:c3:0d:7f > 00:1a:11:c3:0d:7f, ethertype IPv6 (0x86dd), length 118: 4444::555:23 > 4444::555:27: ICMP6, echo request, seq 1, length 64
02:12:16.698249 00:1a:11:c3:0d:7f > 00:1a:11:c3:0d:7f, ethertype IPv6 (0x86dd), length 118: 4444::555:23 > 4444::555:27: ICMP6, echo request, seq 2, length 64
02:12:17.698224 00:1a:11:c3:0d:7f > 00:1a:11:c3:0d:7f, ethertype IPv6 (0x86dd), length 118: 4444::555:23 > 4444::555:27: ICMP6, echo request, seq 3, length 64
^C
--- 4444::555:0027 ping statistics ---
3 packets transmitted, 0 received, 100% packet loss, time 1999ms

lpaa23:~# ip -6 neigh flush dev eth0

lpaa23:~# ip -6 neigh sh dev eth0
4444::555:24  FAILED
4444::555:28  FAILED
fe80::21a:11ff:fec3:d45  FAILED
4444::555:26  FAILED
4444::555:25  FAILED

Oh well...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] arp: filter NOARP neighbours for SIOCGARP
  2015-07-29  9:15   ` Eric Dumazet
@ 2015-07-29  9:32     ` Eric Dumazet
  2015-07-29 10:01       ` [PATCH net] ipv6: flush nd cache on IFF_NOARP change Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2015-07-29  9:32 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, valas, willemb

On Wed, 2015-07-29 at 11:15 +0200, Eric Dumazet wrote:
> On Tue, 2015-07-28 at 23:41 -0700, David Miller wrote:
> 
> > Applied, thanks Eric.
> Thanks David
> 
> Note that IPv6 is completely broken after 'arp off' sequence.

It seems we need to replicate what commit
6c8b4e3ff81b82fc153625e81e60af1d89de2c32 ("arp: flush arp cache on
IFF_NOARP change") 
did for IPv4

Will test this and submit an official patch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net] ipv6: flush nd cache on IFF_NOARP change
  2015-07-29  9:32     ` Eric Dumazet
@ 2015-07-29 10:01       ` Eric Dumazet
  2015-07-30  6:01         ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2015-07-29 10:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, valas, willemb, Mario Fanelli, edumazet

From: Eric Dumazet <edumazet@google.com>

This patch is the IPv6 equivalent of commit
6c8b4e3ff81b ("arp: flush arp cache on IFF_NOARP change") 

Without it, we keep buggy neighbours in the cache, with destination
MAC address equal to our own MAC address.

Tested:
 tcpdump -i eth0 -s 0 ip6 -n -e &
 ip link set dev eth0 arp off
 ping6 remote   // sends buggy frames
 ip link set dev eth0 arp on
 ping6 remote   // should work once kernel is patched

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Mario Fanelli <mariofanelli@google.com>
---
 net/ipv6/ndisc.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 0a05b35..c53331c 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1650,6 +1650,7 @@ int ndisc_rcv(struct sk_buff *skb)
 static int ndisc_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+	struct netdev_notifier_change_info *change_info;
 	struct net *net = dev_net(dev);
 	struct inet6_dev *idev;
 
@@ -1664,6 +1665,11 @@ static int ndisc_netdev_event(struct notifier_block *this, unsigned long event,
 			ndisc_send_unsol_na(dev);
 		in6_dev_put(idev);
 		break;
+	case NETDEV_CHANGE:
+		change_info = ptr;
+		if (change_info->flags_changed & IFF_NOARP)
+			neigh_changeaddr(&nd_tbl, dev);
+		break;
 	case NETDEV_DOWN:
 		neigh_ifdown(&nd_tbl, dev);
 		fib6_run_gc(0, net, false);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net] ipv6: flush nd cache on IFF_NOARP change
  2015-07-29 10:01       ` [PATCH net] ipv6: flush nd cache on IFF_NOARP change Eric Dumazet
@ 2015-07-30  6:01         ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2015-07-30  6:01 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, valas, willemb, mariofanelli, edumazet

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 29 Jul 2015 12:01:41 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> This patch is the IPv6 equivalent of commit
> 6c8b4e3ff81b ("arp: flush arp cache on IFF_NOARP change") 
> 
> Without it, we keep buggy neighbours in the cache, with destination
> MAC address equal to our own MAC address.
> 
> Tested:
>  tcpdump -i eth0 -s 0 ip6 -n -e &
>  ip link set dev eth0 arp off
>  ping6 remote   // sends buggy frames
>  ip link set dev eth0 arp on
>  ping6 remote   // should work once kernel is patched
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Mario Fanelli <mariofanelli@google.com>

Applied, thanks Eric.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-07-30  6:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-27  9:33 [PATCH net] arp: filter NOARP neighbours for SIOCGARP Eric Dumazet
2015-07-29  6:41 ` David Miller
2015-07-29  9:15   ` Eric Dumazet
2015-07-29  9:32     ` Eric Dumazet
2015-07-29 10:01       ` [PATCH net] ipv6: flush nd cache on IFF_NOARP change Eric Dumazet
2015-07-30  6:01         ` 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).