dev_addr: add a modification check
authorJakub Kicinski <kuba@kernel.org>
Fri, 19 Nov 2021 14:21:53 +0000 (06:21 -0800)
committerDavid S. Miller <davem@davemloft.net>
Sat, 20 Nov 2021 12:25:57 +0000 (12:25 +0000)
netdev->dev_addr should only be modified via helpers,
but someone may be casting off the const. Add a runtime
check to catch abuses.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/linux/netdevice.h
net/core/dev.c
net/core/dev_addr_lists.c

index 2462195..cb7f266 100644 (file)
@@ -1942,6 +1942,8 @@ enum netdev_ml_priv_type {
  *     @unlink_list:   As netif_addr_lock() can be called recursively,
  *                     keep a list of interfaces to be deleted.
  *
+ *     @dev_addr_shadow:       Copy of @dev_addr to catch direct writes.
+ *
  *     FIXME: cleanup struct net_device such that network protocol info
  *     moves out.
  */
@@ -2268,6 +2270,8 @@ struct net_device {
 
        /* protected by rtnl_lock */
        struct bpf_xdp_entity   xdp_state[__MAX_XDP_MODE];
+
+       u8 dev_addr_shadow[MAX_ADDR_LEN];
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
@@ -4288,6 +4292,7 @@ int dev_addr_del(struct net_device *dev, const unsigned char *addr,
                 unsigned char addr_type);
 void dev_addr_flush(struct net_device *dev);
 int dev_addr_init(struct net_device *dev);
+void dev_addr_check(struct net_device *dev);
 
 /* Functions used for unicast addresses handling */
 int dev_uc_add(struct net_device *dev, const unsigned char *addr);
index 92c9258..9219e31 100644 (file)
@@ -1377,6 +1377,7 @@ static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack)
        int ret;
 
        ASSERT_RTNL();
+       dev_addr_check(dev);
 
        if (!netif_device_present(dev)) {
                /* may be detached because parent is runtime-suspended */
index a23a83a..9699427 100644 (file)
@@ -498,6 +498,21 @@ EXPORT_SYMBOL(__hw_addr_init);
  * Device addresses handling functions
  */
 
+/* Check that netdev->dev_addr is not written to directly as this would
+ * break the rbtree layout. All changes should go thru dev_addr_set() and co.
+ * Remove this check in mid-2024.
+ */
+void dev_addr_check(struct net_device *dev)
+{
+       if (!memcmp(dev->dev_addr, dev->dev_addr_shadow, MAX_ADDR_LEN))
+               return;
+
+       netdev_warn(dev, "Current addr:  %*ph\n", MAX_ADDR_LEN, dev->dev_addr);
+       netdev_warn(dev, "Expected addr: %*ph\n",
+                   MAX_ADDR_LEN, dev->dev_addr_shadow);
+       netdev_WARN(dev, "Incorrect netdev->dev_addr\n");
+}
+
 /**
  *     dev_addr_flush - Flush device address list
  *     @dev: device
@@ -509,6 +524,7 @@ EXPORT_SYMBOL(__hw_addr_init);
 void dev_addr_flush(struct net_device *dev)
 {
        /* rtnl_mutex must be held here */
+       dev_addr_check(dev);
 
        __hw_addr_flush(&dev->dev_addrs);
        dev->dev_addr = NULL;
@@ -552,8 +568,11 @@ void dev_addr_mod(struct net_device *dev, unsigned int offset,
 {
        struct netdev_hw_addr *ha;
 
+       dev_addr_check(dev);
+
        ha = container_of(dev->dev_addr, struct netdev_hw_addr, addr[0]);
        memcpy(&ha->addr[offset], addr, len);
+       memcpy(&dev->dev_addr_shadow[offset], addr, len);
 }
 EXPORT_SYMBOL(dev_addr_mod);