From: Jakub Kicinski <kuba@kernel•org>
To: davem@davemloft•net
Cc: netdev@vger•kernel.org, Jakub Kicinski <kuba@kernel•org>
Subject: [PATCH net-next v2 5/7] dev_addr: add a modification check
Date: Fri, 19 Nov 2021 06:21:53 -0800 [thread overview]
Message-ID: <20211119142155.3779933-6-kuba@kernel.org> (raw)
In-Reply-To: <20211119142155.3779933-1-kuba@kernel.org>
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>
---
include/linux/netdevice.h | 5 +++++
net/core/dev.c | 1 +
net/core/dev_addr_lists.c | 19 +++++++++++++++++++
3 files changed, 25 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2462195784a9..cb7f2661d187 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -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);
diff --git a/net/core/dev.c b/net/core/dev.c
index 92c9258cbf28..9219e319e901 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -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 */
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index a23a83ac18e5..969942734951 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -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);
--
2.31.1
next prev parent reply other threads:[~2021-11-19 14:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-19 14:21 [PATCH net-next v2 0/7] net: constify netdev->dev_addr Jakub Kicinski
2021-11-19 14:21 ` [PATCH net-next v2 1/7] 82596: use eth_hw_addr_set() Jakub Kicinski
2021-11-19 14:21 ` [PATCH net-next v2 2/7] bnx2x: constify static inline stub for dev_addr Jakub Kicinski
2021-11-19 14:21 ` [PATCH net-next v2 3/7] net: constify netdev->dev_addr Jakub Kicinski
2021-11-19 14:21 ` [PATCH net-next v2 4/7] net: unexport dev_addr_init() & dev_addr_flush() Jakub Kicinski
2021-11-19 14:21 ` Jakub Kicinski [this message]
2021-11-19 14:21 ` [PATCH net-next v2 6/7] dev_addr_list: put the first addr on the tree Jakub Kicinski
2021-11-19 14:21 ` [PATCH net-next v2 7/7] net: kunit: add a test for dev_addr_lists Jakub Kicinski
2021-11-19 15:46 ` [PATCH net-next v2 0/7] net: constify netdev->dev_addr David Miller
2021-11-20 12:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211119142155.3779933-6-kuba@kernel.org \
--to=kuba@kernel$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox