From: Stefan Wiehler <stefan.wiehler@nokia.com>
To: "David S . Miller" <davem@davemloft.net>,
David Ahern <dsahern@kernel.org>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Stefan Wiehler <stefan.wiehler@nokia.com>,
Petr Malat <oss@malat.biz>
Subject: [PATCH net v2] ip6mr: Fix lockdep and sparse RCU warnings
Date: Tue, 1 Oct 2024 12:01:20 +0200 [thread overview]
Message-ID: <20241001100119.230711-2-stefan.wiehler@nokia.com> (raw)
ip6mr_vif_seq_start() must lock RCU even in a case of error, because
stop callback is called unconditionally.
When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
should be done under RCU or RTNL lock. Lock RCU before the call unless
it's done already or RTNL lock is held.
Signed-off-by: Petr Malat <oss@malat.biz>
Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
---
v2:
- rebase on top of net tree
- add Fixes tag
- refactor out paths
v1: https://patchwork.kernel.org/project/netdevbpf/patch/20240605195355.363936-1-oss@malat.biz/
---
net/ipv6/ip6mr.c | 103 +++++++++++++++++++++++++++++------------------
1 file changed, 64 insertions(+), 39 deletions(-)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 2ce4ae0d8dc3..9a72928861ac 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -411,13 +411,13 @@ static void *ip6mr_vif_seq_start(struct seq_file *seq, loff_t *pos)
struct net *net = seq_file_net(seq);
struct mr_table *mrt;
+ rcu_read_lock();
mrt = ip6mr_get_table(net, RT6_TABLE_DFLT);
if (!mrt)
return ERR_PTR(-ENOENT);
iter->mrt = mrt;
- rcu_read_lock();
return mr_vif_seq_start(seq, pos);
}
@@ -1884,18 +1884,23 @@ int ip6mr_ioctl(struct sock *sk, int cmd, void *arg)
struct mfc6_cache *c;
struct net *net = sock_net(sk);
struct mr_table *mrt;
+ int err;
+ rcu_read_lock();
mrt = ip6mr_get_table(net, raw6_sk(sk)->ip6mr_table ? : RT6_TABLE_DFLT);
- if (!mrt)
- return -ENOENT;
+ if (!mrt) {
+ err = -ENOENT;
+ goto out;
+ }
switch (cmd) {
case SIOCGETMIFCNT_IN6:
vr = (struct sioc_mif_req6 *)arg;
- if (vr->mifi >= mrt->maxvif)
- return -EINVAL;
+ if (vr->mifi >= mrt->maxvif) {
+ err = -EINVAL;
+ goto out;
+ }
vr->mifi = array_index_nospec(vr->mifi, mrt->maxvif);
- rcu_read_lock();
vif = &mrt->vif_table[vr->mifi];
if (VIF_EXISTS(mrt, vr->mifi)) {
vr->icount = READ_ONCE(vif->pkt_in);
@@ -1905,12 +1910,11 @@ int ip6mr_ioctl(struct sock *sk, int cmd, void *arg)
rcu_read_unlock();
return 0;
}
- rcu_read_unlock();
- return -EADDRNOTAVAIL;
+ err = -EADDRNOTAVAIL;
+ goto out;
case SIOCGETSGCNT_IN6:
sr = (struct sioc_sg_req6 *)arg;
- rcu_read_lock();
c = ip6mr_cache_find(mrt, &sr->src.sin6_addr,
&sr->grp.sin6_addr);
if (c) {
@@ -1920,11 +1924,16 @@ int ip6mr_ioctl(struct sock *sk, int cmd, void *arg)
rcu_read_unlock();
return 0;
}
- rcu_read_unlock();
- return -EADDRNOTAVAIL;
+ err = -EADDRNOTAVAIL;
+ goto out;
default:
- return -ENOIOCTLCMD;
+ err = -ENOIOCTLCMD;
+ goto out;
}
+
+out:
+ rcu_read_unlock();
+ return err;
}
#ifdef CONFIG_COMPAT
@@ -1952,19 +1961,35 @@ int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
struct mfc6_cache *c;
struct net *net = sock_net(sk);
struct mr_table *mrt;
-
- mrt = ip6mr_get_table(net, raw6_sk(sk)->ip6mr_table ? : RT6_TABLE_DFLT);
- if (!mrt)
- return -ENOENT;
+ int err;
switch (cmd) {
case SIOCGETMIFCNT_IN6:
if (copy_from_user(&vr, arg, sizeof(vr)))
return -EFAULT;
- if (vr.mifi >= mrt->maxvif)
- return -EINVAL;
+ break;
+ case SIOCGETSGCNT_IN6:
+ if (copy_from_user(&sr, arg, sizeof(sr)))
+ return -EFAULT;
+ break;
+ default:
+ return -ENOIOCTLCMD;
+ }
+
+ rcu_read_lock();
+ mrt = ip6mr_get_table(net, raw6_sk(sk)->ip6mr_table ? : RT6_TABLE_DFLT);
+ if (!mrt) {
+ err = -ENOENT;
+ goto out;
+ }
+
+ switch (cmd) {
+ case SIOCGETMIFCNT_IN6:
+ if (vr.mifi >= mrt->maxvif) {
+ err = -EINVAL;
+ goto out;
+ }
vr.mifi = array_index_nospec(vr.mifi, mrt->maxvif);
- rcu_read_lock();
vif = &mrt->vif_table[vr.mifi];
if (VIF_EXISTS(mrt, vr.mifi)) {
vr.icount = READ_ONCE(vif->pkt_in);
@@ -1977,13 +2002,9 @@ int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
return -EFAULT;
return 0;
}
- rcu_read_unlock();
- return -EADDRNOTAVAIL;
+ err = -EADDRNOTAVAIL;
+ goto out;
case SIOCGETSGCNT_IN6:
- if (copy_from_user(&sr, arg, sizeof(sr)))
- return -EFAULT;
-
- rcu_read_lock();
c = ip6mr_cache_find(mrt, &sr.src.sin6_addr, &sr.grp.sin6_addr);
if (c) {
sr.pktcnt = c->_c.mfc_un.res.pkt;
@@ -1995,11 +2016,13 @@ int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
return -EFAULT;
return 0;
}
- rcu_read_unlock();
- return -EADDRNOTAVAIL;
- default:
- return -ENOIOCTLCMD;
+ err = -EADDRNOTAVAIL;
+ goto out;
}
+
+out:
+ rcu_read_unlock();
+ return err;
}
#endif
@@ -2275,11 +2298,13 @@ int ip6mr_get_route(struct net *net, struct sk_buff *skb, struct rtmsg *rtm,
struct mfc6_cache *cache;
struct rt6_info *rt = dst_rt6_info(skb_dst(skb));
+ rcu_read_lock();
mrt = ip6mr_get_table(net, RT6_TABLE_DFLT);
- if (!mrt)
- return -ENOENT;
+ if (!mrt) {
+ err = -ENOENT;
+ goto out;
+ }
- rcu_read_lock();
cache = ip6mr_cache_find(mrt, &rt->rt6i_src.addr, &rt->rt6i_dst.addr);
if (!cache && skb->dev) {
int vif = ip6mr_find_vif(mrt, skb->dev);
@@ -2297,15 +2322,15 @@ int ip6mr_get_route(struct net *net, struct sk_buff *skb, struct rtmsg *rtm,
dev = skb->dev;
if (!dev || (vif = ip6mr_find_vif(mrt, dev)) < 0) {
- rcu_read_unlock();
- return -ENODEV;
+ err = -ENODEV;
+ goto out;
}
/* really correct? */
skb2 = alloc_skb(sizeof(struct ipv6hdr), GFP_ATOMIC);
if (!skb2) {
- rcu_read_unlock();
- return -ENOMEM;
+ err = -ENOMEM;
+ goto out;
}
NETLINK_CB(skb2).portid = portid;
@@ -2327,12 +2352,12 @@ int ip6mr_get_route(struct net *net, struct sk_buff *skb, struct rtmsg *rtm,
iph->daddr = rt->rt6i_dst.addr;
err = ip6mr_cache_unresolved(mrt, vif, skb2, dev);
- rcu_read_unlock();
-
- return err;
+ goto out;
}
err = mr_fill_mroute(mrt, skb, &cache->_c, rtm);
+
+out:
rcu_read_unlock();
return err;
}
--
2.42.0
next reply other threads:[~2024-10-01 10:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-01 10:01 Stefan Wiehler [this message]
2024-10-01 10:34 ` [PATCH net v2] ip6mr: Fix lockdep and sparse RCU warnings Eric Dumazet
2024-10-01 12:50 ` Stefan Wiehler
2024-10-01 13:26 ` Eric Dumazet
2024-10-01 14:36 ` Stefan Wiehler
2024-10-01 14:39 ` ericnetdev dumazet
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=20241001100119.230711-2-stefan.wiehler@nokia.com \
--to=stefan.wiehler@nokia.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=oss@malat.biz \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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