netdev - [PATCH v2 net-next] arp: flush arp cache on IFF

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

Date:	Tue, 21 May 2013 13:23:44 +0300
From:	Timo Teräs <timo.teras@....fi>
To:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	kaber@...sh.net
Cc:	Timo Teräs <timo.teras@....fi>
Subject: [PATCH v2 net-next] arp: flush arp cache on IFF_NOARP change

IFF_NOARP affects what kind of neighbor entries are created
(nud NOARP or nud INCOMPLETE). If the flag changes, flush the arp
cache to refresh all entries.

Signed-off-by: Timo Teräs <timo.teras@....fi>
---
> This patch makes no sense at all.
> 
> The state bit in ->priv_flags is a boolean stating whether the
> notified should do something or not.
> 
> But you're setting it to match what IFF_NOARP is.
>
> You should set it any time IFF_NOARP _changes_, and then clear
> the bit when the notifier clears the neighbour entries.

IFF_NOARP_CHANGED is set according to "changes = dev->flags ^ old_flags;"
which reflect the change. But I agree that the clearing out bit was
misplaced. This is especially true as it seems NETDEV_CHANGE can be
notified from another place too.

I've updated the if.h comment to state that the bit is valid only during
NETDEV_CHANGE notifier. And __dev_notify_flags is updated to always clear
the bit after notifiers are done.

 include/uapi/linux/if.h | 2 ++
 net/core/dev.c          | 6 +++++-
 net/ipv4/arp.c          | 4 ++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 1ec407b..1be8b35 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -83,6 +83,8 @@
 #define IFF_SUPP_NOFCS	0x80000		/* device supports sending custom FCS */
 #define IFF_LIVE_ADDR_CHANGE 0x100000	/* device supports hardware address
 					 * change when it's running */
+#define IFF_NOARP_CHANGED 0x200000	/* Set during NETDEV_CHANGE notifier
+					 * if IFF_NOARP has changed */
 
 
 #define IF_GET_IFACE	0x0001		/* for querying only */
diff --git a/net/core/dev.c b/net/core/dev.c
index 18e9730..ce30761 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4699,8 +4699,12 @@ void __dev_notify_flags(struct net_device *dev, unsigned int old_flags)
 	}
 
 	if (dev->flags & IFF_UP &&
-	    (changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE)))
+	    (changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE))) {
+		if (changes & IFF_NOARP)
+			dev->priv_flags |= IFF_NOARP_CHANGED;
 		call_netdevice_notifiers(NETDEV_CHANGE, dev);
+		dev->priv_flags &= ~IFF_NOARP_CHANGED;
+	}
 }
 
 /**
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 247ec19..375b2f2 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1241,6 +1241,10 @@ static int arp_netdev_event(struct notifier_block *this, unsigned long event,
 		neigh_changeaddr(&arp_tbl, dev);
 		rt_cache_flush(dev_net(dev));
 		break;
+	case NETDEV_CHANGE:
+		if (dev->priv_flags & IFF_NOARP_CHANGED)
+			neigh_changeaddr(&arp_tbl, dev);
+		break;
 	default:
 		break;
 	}
-- 
1.8.2.3

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html