dahdi-linux
dahdi-linux copied to clipboard
Linux 5.17+: remove direct writes to `dev_addr`
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 <[email protected]>
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;
This issue doesn't reproduce anymore, and it looks like the "dodgy workaround" (as @john-tho called it: https://github.com/openwrt/telephony/pull/786#issuecomment-1637309661) was implemented in https://github.com/asterisk/dahdi-linux/commit/63a2657fdc5c838809ef57ff6da18721d8ba04b2
So, from a build perspective, this issue has been resolved, although I suppose reasonable developers can disagree whether that was the best approach or not...