Skip to content

Commit

Permalink
ipv4: fix a potential deadlock in mcast getsockopt() path
Browse files Browse the repository at this point in the history
Sasha reported the following lockdep warning:

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(sk_lock-AF_INET);
                                lock(rtnl_mutex);
                                lock(sk_lock-AF_INET);
   lock(rtnl_mutex);

This is due to that for IP_MSFILTER and MCAST_MSFILTER, we take
rtnl lock before the socket lock in setsockopt() path, but take
the socket lock before rtnl lock in getsockopt() path. All the
rest optnames are setsockopt()-only.

Fix this by aligning the getsockopt() path with the setsockopt()
path, so that all mcast socket path would be locked in the same
order.

Note, IPv6 part is different where rtnl lock is not held.

Fixes: 54ff9ef ("ipv4, ipv6: kill ip_mc_{join, leave}_group and ipv6_sock_mc_{join, drop}")
Reported-by: Sasha Levin <[email protected]>
Cc: Marcelo Ricardo Leitner <[email protected]>
Signed-off-by: Cong Wang <[email protected]>
Reviewed-by: Marcelo Ricardo Leitner <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
congwang authored and davem330 committed Nov 5, 2015
1 parent 4ee3bd4 commit 87e9f03
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 23 deletions.
12 changes: 4 additions & 8 deletions net/ipv4/igmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2392,11 +2392,11 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
struct ip_sf_socklist *psl;
struct net *net = sock_net(sk);

ASSERT_RTNL();

if (!ipv4_is_multicast(addr))
return -EINVAL;

rtnl_lock();

imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
imr.imr_address.s_addr = msf->imsf_interface;
imr.imr_ifindex = 0;
Expand All @@ -2417,7 +2417,6 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
goto done;
msf->imsf_fmode = pmc->sfmode;
psl = rtnl_dereference(pmc->sflist);
rtnl_unlock();
if (!psl) {
len = 0;
count = 0;
Expand All @@ -2436,7 +2435,6 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
return -EFAULT;
return 0;
done:
rtnl_unlock();
return err;
}

Expand All @@ -2450,15 +2448,15 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
struct inet_sock *inet = inet_sk(sk);
struct ip_sf_socklist *psl;

ASSERT_RTNL();

psin = (struct sockaddr_in *)&gsf->gf_group;
if (psin->sin_family != AF_INET)
return -EINVAL;
addr = psin->sin_addr.s_addr;
if (!ipv4_is_multicast(addr))
return -EINVAL;

rtnl_lock();

err = -EADDRNOTAVAIL;

for_each_pmc_rtnl(inet, pmc) {
Expand All @@ -2470,7 +2468,6 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
goto done;
gsf->gf_fmode = pmc->sfmode;
psl = rtnl_dereference(pmc->sflist);
rtnl_unlock();
count = psl ? psl->sl_count : 0;
copycount = count < gsf->gf_numsrc ? count : gsf->gf_numsrc;
gsf->gf_numsrc = count;
Expand All @@ -2490,7 +2487,6 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
}
return 0;
done:
rtnl_unlock();
return err;
}

Expand Down
45 changes: 30 additions & 15 deletions net/ipv4/ip_sockglue.c
Original file line number Diff line number Diff line change
Expand Up @@ -1251,11 +1251,22 @@ EXPORT_SYMBOL(compat_ip_setsockopt);
* the _received_ ones. The set sets the _sent_ ones.
*/

static bool getsockopt_needs_rtnl(int optname)
{
switch (optname) {
case IP_MSFILTER:
case MCAST_MSFILTER:
return true;
}
return false;
}

static int do_ip_getsockopt(struct sock *sk, int level, int optname,
char __user *optval, int __user *optlen, unsigned int flags)
{
struct inet_sock *inet = inet_sk(sk);
int val;
bool needs_rtnl = getsockopt_needs_rtnl(optname);
int val, err = 0;
int len;

if (level != SOL_IP)
Expand All @@ -1269,6 +1280,8 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
if (len < 0)
return -EINVAL;

if (needs_rtnl)
rtnl_lock();
lock_sock(sk);

switch (optname) {
Expand Down Expand Up @@ -1386,39 +1399,35 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
case IP_MSFILTER:
{
struct ip_msfilter msf;
int err;

if (len < IP_MSFILTER_SIZE(0)) {
release_sock(sk);
return -EINVAL;
err = -EINVAL;
goto out;
}
if (copy_from_user(&msf, optval, IP_MSFILTER_SIZE(0))) {
release_sock(sk);
return -EFAULT;
err = -EFAULT;
goto out;
}
err = ip_mc_msfget(sk, &msf,
(struct ip_msfilter __user *)optval, optlen);
release_sock(sk);
return err;
goto out;
}
case MCAST_MSFILTER:
{
struct group_filter gsf;
int err;

if (len < GROUP_FILTER_SIZE(0)) {
release_sock(sk);
return -EINVAL;
err = -EINVAL;
goto out;
}
if (copy_from_user(&gsf, optval, GROUP_FILTER_SIZE(0))) {
release_sock(sk);
return -EFAULT;
err = -EFAULT;
goto out;
}
err = ip_mc_gsfget(sk, &gsf,
(struct group_filter __user *)optval,
optlen);
release_sock(sk);
return err;
goto out;
}
case IP_MULTICAST_ALL:
val = inet->mc_all;
Expand Down Expand Up @@ -1485,6 +1494,12 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
return -EFAULT;
}
return 0;

out:
release_sock(sk);
if (needs_rtnl)
rtnl_unlock();
return err;
}

int ip_getsockopt(struct sock *sk, int level,
Expand Down

0 comments on commit 87e9f03

Please sign in to comment.