asterisk / dahdi-linux

This is the official dahdi-linux repository. All issues and PR should be raised here.
GNU General Public License v2.0
47 stars 69 forks source link

Linux 5.17+: remove direct writes to `dev_addr` #31

Open john-tho opened 1 year ago

john-tho commented 1 year ago

Linux 5.17 commits adeef3e32146 ("net: constify netdev->dev_addr") and d07b26f5bbea ("dev_addr: add a modification check") make netdev->dev_addr const, and add a runtime check that it is not being modified by casting off the const.

There are a number of examples of this through the dahdi tree:

https://github.com/asterisk/dahdi-linux/blob/1476d3a43c558dc021d1e2f01ca6131b0dbda2d1/drivers/dahdi/datamods/hdlc_raw_eth.c#L101-L102

https://github.com/asterisk/dahdi-linux/blob/1476d3a43c558dc021d1e2f01ca6131b0dbda2d1/drivers/dahdi/voicebus/voicebus_net.c#L192

https://github.com/asterisk/dahdi-linux/blob/1476d3a43c558dc021d1e2f01ca6131b0dbda2d1/drivers/dahdi/datamods/hdlc_fr.c#L1092-L1095

Example build failure from OpenWrt with kernel 6.1:

                 from dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:24:
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c: In function 'wctc4xxp_net_register':
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:646:22: error: passing argument 1 of '__builtin_memcpy' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
  646 |         memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
      |                ~~~~~~^~~~~~~~~~
./include/linux/fortify-string.h:469:27: note: in definition of macro '__fortify_memcpy_chk'
  469 |         __underlying_##op(p, q, __fortify_size);                        \
      |                           ^
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:646:9: note: in expansion of macro 'memcpy'
  646 |         memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
      |         ^~~~~~
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:646:22: note: expected 'void *' but argument is of type 'const unsigned char *'
  646 |         memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
      |                ~~~~~~^~~~~~~~~~
./include/linux/fortify-string.h:469:27: note: in definition of macro '__fortify_memcpy_chk'
  469 |         __underlying_##op(p, q, __fortify_size);                        \
      |                           ^
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:646:9: note: in expansion of macro 'memcpy'
  646 |         memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
      |         ^~~~~~

From https://github.com/asterisk/dahdi-linux/commit/d51c10dae5a06709983b929fc6f9fb18eec41951, the const is cast off, so would expect to hit the runtime check here: https://github.com/asterisk/dahdi-linux/blob/1476d3a43c558dc021d1e2f01ca6131b0dbda2d1/drivers/dahdi/wctc4xxp/base.c#L646


I am not a dahdi user, only saw that the kernel module was failing to build for OpenWrt kernel 6.1, and wanted to fix that. This was the patch I came up with to address dev_addr writes in dahdi for OpenWrt, but I have not checked that functionality has not changed. I do not currently have the time to rebase, build test, and PR. Feel free to use these suggestions as you see fit. Not completely confident with the drivers/dahdi/datamods/hdlc_fr.c changes. Thank you for working with this git repo once again!

From 3619bf1cc9c0cc67213cbda0db3534f8a64f5cc8 Mon Sep 17 00:00:00 2001
From: John Thomson <git@johnthomson.fastmail.com.au>
Date: Wed, 14 Jun 2023 20:31:42 +1000
Subject: [PATCH 1/1] fix for const dev_addr for linux 5.17

Linux commit adeef3e32146 ("net: constify netdev->dev_addr")

dev_addr_set and _mod were introduced in linux 5.15 with
commit 48eab831ae8b ("net: create netdev->dev_addr assignment helpers")

                 from dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:24:
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c: In function 'wctc4xxp_net_register':
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:646:22: error: passing argument 1 of '__builtin_memcpy' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
  646 |         memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
      |                ~~~~~~^~~~~~~~~~
./include/linux/fortify-string.h:469:27: note: in definition of macro '__fortify_memcpy_chk'
  469 |         __underlying_##op(p, q, __fortify_size);                        \
      |                           ^
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:646:9: note: in expansion of macro 'memcpy'
  646 |         memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
      |         ^~~~~~
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:646:22: note: expected 'void *' but argument is of type 'const unsigned char *'
  646 |         memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
      |                ~~~~~~^~~~~~~~~~
./include/linux/fortify-string.h:469:27: note: in definition of macro '__fortify_memcpy_chk'
  469 |         __underlying_##op(p, q, __fortify_size);                        \
      |                           ^
dahdi-linux-3.2.0/drivers/dahdi/wctc4xxp/base.c:646:9: note: in expansion of macro 'memcpy'
  646 |         memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
      |         ^~~~~~

---
 drivers/dahdi/datamods/hdlc_fr.c      | 6 +++---
 drivers/dahdi/datamods/hdlc_raw_eth.c | 4 ++--
 drivers/dahdi/voicebus/voicebus_net.c | 2 +-
 drivers/dahdi/wctc4xxp/base.c         | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

--- a/drivers/dahdi/datamods/hdlc_fr.c
+++ b/drivers/dahdi/datamods/hdlc_fr.c
@@ -1089,10 +1089,10 @@ static int fr_add_pvc(struct net_device
    }

    if (type == ARPHRD_ETHER) {
-       memcpy(dev->dev_addr, "\x00\x01", 2);
-                get_random_bytes(dev->dev_addr + 2, ETH_ALEN - 2);
+       eth_hw_addr_random(dev);
+       dev_addr_mod(dev, 0, "\x00\x01", 2);
    } else {
-       *(u16*)dev->dev_addr = htons(dlci);
+       dev_addr_set(dev, htons(dlci));
        dlci_to_q922(dev->broadcast, dlci);
    }
    dev->hard_start_xmit = pvc_xmit;
--- a/drivers/dahdi/datamods/hdlc_raw_eth.c
+++ b/drivers/dahdi/datamods/hdlc_raw_eth.c
@@ -98,8 +98,8 @@ int hdlc_raw_eth_ioctl(struct net_device
        ether_setup(dev);
        dev->change_mtu = old_ch_mtu;
        dev->tx_queue_len = old_qlen;
-       memcpy(dev->dev_addr, "\x00\x01", 2);
-                get_random_bytes(dev->dev_addr + 2, ETH_ALEN - 2);
+       eth_hw_addr_random(dev);
+       dev_addr_mod(dev, 0, "\x00\x01", 2);
        return 0;
    }

--- a/drivers/dahdi/voicebus/voicebus_net.c
+++ b/drivers/dahdi/voicebus/voicebus_net.c
@@ -189,7 +189,7 @@ int vb_net_register(struct voicebus *vb,
        return -ENOMEM;
    priv = netdev_priv(netdev);
    priv->vb = vb;
-   memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
+   dev_addr_set(netdev, our_mac);
 #  ifdef HAVE_NET_DEVICE_OPS
    netdev->netdev_ops = &vb_netdev_ops;
 #  else
--- a/drivers/dahdi/wctc4xxp/base.c
+++ b/drivers/dahdi/wctc4xxp/base.c
@@ -643,7 +643,7 @@ wctc4xxp_net_register(struct wcdte *wc)
        return -ENOMEM;
    priv = netdev_priv(netdev);
    priv->wc = wc;
-   memcpy(netdev->dev_addr, our_mac, sizeof(our_mac));
+   dev_addr_set(netdev, our_mac);

 #  ifdef HAVE_NET_DEVICE_OPS
    netdev->netdev_ops = &wctc4xxp_netdev_ops;