[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAA3_GnpjEcK01bJOyxDvDO70E0kqUey8r7W_WukUJzq3=wH+QQ@mail.gmail.com>
Date: Thu, 5 Feb 2026 19:58:00 +0900
From: 戸田晃太 <kota.toda@...-cybersecurity.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: pabeni@...hat.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
小池悠生 <yuki.koike@...-cybersecurity.com>,
戸田晃太 <kota.toda@...-cybersecurity.com>
Subject: [PATCH net 2/2] net: add READ_ONCE for header_ops callbacks
Use READ_ONCE when loading header_ops callbacks to avoid races with
concurrent updates.
Signed-off-by: Kota Toda <kota.toda@...-cybersecurity.com>
Signed-off-by: Yuki Koike <yuki.koike@...-cybersecurity.com>
Co-developed-by: Yuki Koike <yuki.koike@...-cybersecurity.com>
---
include/linux/netdevice.h | 24 +++++++++++++++---------
include/net/cfg802154.h | 2 +-
net/core/neighbour.c | 6 +++---
net/ipv4/arp.c | 2 +-
net/ipv6/ndisc.c | 2 +-
5 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 77a99c8ab..0d2c1c852 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3150,35 +3150,41 @@ static inline int dev_hard_header(struct
sk_buff *skb, struct net_device *dev,
const void *daddr, const void *saddr,
unsigned int len)
{
- if (!dev->header_ops || !dev->header_ops->create)
+ int (*create)(struct sk_buff *skb, struct net_device *dev,
+ unsigned short type, const void *daddr,
+ const void *saddr, unsigned int len);
+ if (!dev->header_ops || !(create = READ_ONCE(dev->header_ops->create)))
return 0;
- return dev->header_ops->create(skb, dev, type, daddr, saddr, len);
+ return create(skb, dev, type, daddr, saddr, len);
}
static inline int dev_parse_header(const struct sk_buff *skb,
unsigned char *haddr)
{
+ int (*parse)(const struct sk_buff *skb, unsigned char *haddr);
const struct net_device *dev = skb->dev;
- if (!dev->header_ops || !dev->header_ops->parse)
+ if (!dev->header_ops || !(parse = READ_ONCE(dev->header_ops->parse)))
return 0;
- return dev->header_ops->parse(skb, haddr);
+ return parse(skb, haddr);
}
static inline __be16 dev_parse_header_protocol(const struct sk_buff *skb)
{
+ __be16 (*parse_protocol)(const struct sk_buff *skb);
const struct net_device *dev = skb->dev;
- if (!dev->header_ops || !dev->header_ops->parse_protocol)
+ if (!dev->header_ops || !(parse_protocol =
READ_ONCE(dev->header_ops->parse_protocol)))
return 0;
- return dev->header_ops->parse_protocol(skb);
+ return parse_protocol(skb);
}
/* ll_header must have at least hard_header_len allocated */
static inline bool dev_validate_header(const struct net_device *dev,
char *ll_header, int len)
{
+ bool (*validate)(const char *ll_header, unsigned int len);
if (likely(len >= dev->hard_header_len))
return true;
if (len < dev->min_header_len)
@@ -3189,15 +3195,15 @@ static inline bool dev_validate_header(const
struct net_device *dev,
return true;
}
- if (dev->header_ops && dev->header_ops->validate)
- return dev->header_ops->validate(ll_header, len);
+ if (dev->header_ops && (validate = READ_ONCE(dev->header_ops->validate)))
+ return validate(ll_header, len);
return false;
}
static inline bool dev_has_header(const struct net_device *dev)
{
- return dev->header_ops && dev->header_ops->create;
+ return dev->header_ops && READ_ONCE(dev->header_ops->create);
}
/*
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 76d2cd2e2..dec638763 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -522,7 +522,7 @@ wpan_dev_hard_header(struct sk_buff *skb, struct
net_device *dev,
{
struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
- return wpan_dev->header_ops->create(skb, dev, daddr, saddr, len);
+ return READ_ONCE(wpan_dev->header_ops->create)(skb, dev, daddr,
saddr, len);
}
#endif
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 96786016d..ff948e35e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1270,7 +1270,7 @@ static void neigh_update_hhs(struct neighbour *neigh)
= NULL;
if (neigh->dev->header_ops)
- update = neigh->dev->header_ops->cache_update;
+ update = READ_ONCE(neigh->dev->header_ops->cache_update);
if (update) {
hh = &neigh->hh;
@@ -1540,7 +1540,7 @@ static void neigh_hh_init(struct neighbour *n)
* hh_cache entry.
*/
if (!hh->hh_len)
- dev->header_ops->cache(n, hh, prot);
+ READ_ONCE(dev->header_ops->cache)(n, hh, prot);
write_unlock_bh(&n->lock);
}
@@ -1556,7 +1556,7 @@ int neigh_resolve_output(struct neighbour
*neigh, struct sk_buff *skb)
struct net_device *dev = neigh->dev;
unsigned int seq;
- if (dev->header_ops->cache && !READ_ONCE(neigh->hh.hh_len))
+ if (READ_ONCE(dev->header_ops->cache) && !READ_ONCE(neigh->hh.hh_len))
neigh_hh_init(neigh);
do {
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7822b2144..421bea6eb 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -278,7 +278,7 @@ static int arp_constructor(struct neighbour *neigh)
memcpy(neigh->ha, dev->broadcast, dev->addr_len);
}
- if (dev->header_ops->cache)
+ if (READ_ONCE(dev->header_ops->cache))
neigh->ops = &arp_hh_ops;
else
neigh->ops = &arp_generic_ops;
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index d961e6c2d..d81f509ec 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -361,7 +361,7 @@ static int ndisc_constructor(struct neighbour *neigh)
neigh->nud_state = NUD_NOARP;
memcpy(neigh->ha, dev->broadcast, dev->addr_len);
}
- if (dev->header_ops->cache)
+ if (READ_ONCE(dev->header_ops->cache))
neigh->ops = &ndisc_hh_ops;
else
neigh->ops = &ndisc_generic_ops;
--
2.53.0
2026年2月3日(火) 2:11 Eric Dumazet <edumazet@...gle.com>:
>
> On Wed, Jan 28, 2026 at 11:46 AM 戸田晃太 <kota.toda@...-cybersecurity.com> wrote:
> >
> > Here is the patch with the barriers added, based on v6.12.67.
> >
> > However, as Yuki said, we are wondering if this would be considered an
> > acceptable change
> > from the perspective of the maintainers (or in terms of Linux kernel
> > culture). This is because
> > the patch adds `READ_ONCE` to several locations outside of bonding subsystem.
> > Please let me know if you have any concerns regarding this point.
> >
> > > Also, please clarify what happens with stacks of two or more bonding devices ?
> >
> > To clarify, currently the `header_ops` of the bottom-most
> > interface are used regardless of the number of bonding layers.
> > This patch changes it so that `&bond->bond_header_ops` is used
> > as the bond device's `header_ops`, regardless of the stack depth.
>
> Could you try to cook a patch series perhaps ?
>
> The READ_ONCE()/WRITE_ONCE() on dev->header_ops->cache could be done separately.
>
> Thanks.
Powered by blists - more mailing lists