diff mbox series

[net,v3,3/4] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_compat_ioctl()

Message ID 20241010090741.1980100-7-stefan.wiehler@nokia.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net,v3,1/4] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_vif_seq_start() | expand

Checks

Context Check Description
netdev/series_format warning Series does not have a cover letter
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 6 this patch: 6
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 5 of 6 maintainers
netdev/build_clang fail Errors and warnings before: 6 this patch: 9
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn fail Errors and warnings before: 6 this patch: 7
netdev/checkpatch warning CHECK: Please don't use multiple blank lines
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Stefan Wiehler Oct. 10, 2024, 9:07 a.m. UTC 
When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
must be done under RCU or RTNL lock. Copy from user space must be
performed beforehand as we are not allowed to sleep under RCU lock.

Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
---
v3:
  - split into separate patches
v2: https://patchwork.kernel.org/project/netdevbpf/patch/20241001100119.230711-2-stefan.wiehler@nokia.com/
  - 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 | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)

Comments

Simon Horman Oct. 10, 2024, 9:41 a.m. UTC | #1 
On Thu, Oct 10, 2024 at 11:07:44AM +0200, Stefan Wiehler wrote:
> When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
> must be done under RCU or RTNL lock. Copy from user space must be
> performed beforehand as we are not allowed to sleep under RCU lock.
> 
> Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
> Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
> ---
> v3:
>   - split into separate patches
> v2: https://patchwork.kernel.org/project/netdevbpf/patch/20241001100119.230711-2-stefan.wiehler@nokia.com/
>   - 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 | 46 ++++++++++++++++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index b18eb4ad21e4..415ba6f55a44 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -1961,10 +1961,7 @@ 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:
> @@ -1972,8 +1969,30 @@ int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
>  			return -EFAULT;
>  		if (vr.mifi >= mrt->maxvif)
>  			return -EINVAL;

Hi Stefan,

mrt is now used uninitialised here.

> +		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);

...

> @@ -2004,11 +2020,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;
>  	}
> +

I think that this out label should be used consistently once rcu_read_lock
has been taken. With this patch applied there seems to be one case on error
where rcu_read_unlock() before returning, and one case where it isn't
(which looks like it leaks the lock).

> +out:
> +	rcu_read_unlock();
> +	return err;
>  }
>  #endif
Stefan Wiehler Oct. 10, 2024, 2:43 p.m. UTC | #2 
>> When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
>> must be done under RCU or RTNL lock. Copy from user space must be
>> performed beforehand as we are not allowed to sleep under RCU lock.
>>
>> Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
>> Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
>> ---
>> v3:
>>   - split into separate patches
>> v2: https://patchwork.kernel.org/project/netdevbpf/patch/20241001100119.230711-2-stefan.wiehler@nokia.com/
>>   - 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 | 46 ++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 32 insertions(+), 14 deletions(-)
>>
>> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
>> index b18eb4ad21e4..415ba6f55a44 100644
>> --- a/net/ipv6/ip6mr.c
>> +++ b/net/ipv6/ip6mr.c
>> @@ -1961,10 +1961,7 @@ 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:
>> @@ -1972,8 +1969,30 @@ int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
>>                       return -EFAULT;
>>               if (vr.mifi >= mrt->maxvif)
>>                       return -EINVAL;
> 
> Hi Stefan,
> 
> mrt is now used uninitialised here.

Thanks, that was an accident, it should have stayed where it is.

>> +             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);
> 
> ...
> 
>> @@ -2004,11 +2020,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;
>>       }
>> +
> 
> I think that this out label should be used consistently once rcu_read_lock
> has been taken. With this patch applied there seems to be one case on error
> where rcu_read_unlock() before returning, and one case where it isn't
> (which looks like it leaks the lock).

In the remaining two return paths we need to release the RCU lock before
calling copy_to_user(), so unfortunately we cannot use a common out label.

Kind regards,

Stefan
Simon Horman Oct. 11, 2024, 10:16 a.m. UTC | #3 
On Thu, Oct 10, 2024 at 04:43:34PM +0200, Stefan Wiehler wrote:
> >> When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
> >> must be done under RCU or RTNL lock. Copy from user space must be
> >> performed beforehand as we are not allowed to sleep under RCU lock.
> >>
> >> Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
> >> Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")

...

> >> @@ -2004,11 +2020,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;
> >>       }
> >> +
> > 
> > I think that this out label should be used consistently once rcu_read_lock
> > has been taken. With this patch applied there seems to be one case on error
> > where rcu_read_unlock() before returning, and one case where it isn't
> > (which looks like it leaks the lock).
> 
> In the remaining two return paths we need to release the RCU lock before
> calling copy_to_user(), so unfortunately we cannot use a common out label.

Ok, sure. But can you check that the code always releases the lock?
kernel test robot Oct. 11, 2024, 5:11 p.m. UTC | #4 
Hi Stefan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Stefan-Wiehler/ip6mr-Lock-RCU-before-ip6mr_get_table-call-in-ip6mr_ioctl/20241010-171634
base:   net/main
patch link:    https://lore.kernel.org/r/20241010090741.1980100-7-stefan.wiehler%40nokia.com
patch subject: [PATCH net v3 3/4] ip6mr: Lock RCU before ip6mr_get_table() call in ip6mr_compat_ioctl()
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20241012/202410120109.QyXpPs23-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241012/202410120109.QyXpPs23-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410120109.QyXpPs23-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/ipv6/ip6mr.c:1970:18: warning: variable 'mrt' is uninitialized when used here [-Wuninitialized]
    1970 |                 if (vr.mifi >= mrt->maxvif)
         |                                ^~~
   net/ipv6/ip6mr.c:1963:22: note: initialize the variable 'mrt' to silence this warning
    1963 |         struct mr_table *mrt;
         |                             ^
         |                              = NULL
   1 warning generated.


vim +/mrt +1970 net/ipv6/ip6mr.c

e2d57766e6744f David S. Miller   2011-02-03  1955  
e2d57766e6744f David S. Miller   2011-02-03  1956  int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
e2d57766e6744f David S. Miller   2011-02-03  1957  {
e2d57766e6744f David S. Miller   2011-02-03  1958  	struct compat_sioc_sg_req6 sr;
e2d57766e6744f David S. Miller   2011-02-03  1959  	struct compat_sioc_mif_req6 vr;
6853f21f764b04 Yuval Mintz       2018-02-28  1960  	struct vif_device *vif;
e2d57766e6744f David S. Miller   2011-02-03  1961  	struct mfc6_cache *c;
e2d57766e6744f David S. Miller   2011-02-03  1962  	struct net *net = sock_net(sk);
b70432f7319eb7 Yuval Mintz       2018-02-28  1963  	struct mr_table *mrt;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1964  	int err;
e2d57766e6744f David S. Miller   2011-02-03  1965  
e2d57766e6744f David S. Miller   2011-02-03  1966  	switch (cmd) {
e2d57766e6744f David S. Miller   2011-02-03  1967  	case SIOCGETMIFCNT_IN6:
e2d57766e6744f David S. Miller   2011-02-03  1968  		if (copy_from_user(&vr, arg, sizeof(vr)))
e2d57766e6744f David S. Miller   2011-02-03  1969  			return -EFAULT;
e2d57766e6744f David S. Miller   2011-02-03 @1970  		if (vr.mifi >= mrt->maxvif)
e2d57766e6744f David S. Miller   2011-02-03  1971  			return -EINVAL;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1972  		break;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1973  	case SIOCGETSGCNT_IN6:
9135a3aca0c10d Stefan Wiehler    2024-10-10  1974  		if (copy_from_user(&sr, arg, sizeof(sr)))
9135a3aca0c10d Stefan Wiehler    2024-10-10  1975  			return -EFAULT;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1976  		break;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1977  	default:
9135a3aca0c10d Stefan Wiehler    2024-10-10  1978  		return -ENOIOCTLCMD;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1979  	}
9135a3aca0c10d Stefan Wiehler    2024-10-10  1980  
9135a3aca0c10d Stefan Wiehler    2024-10-10  1981  
638cf4a24a09d1 Eric Dumazet      2022-06-23  1982  	rcu_read_lock();
9135a3aca0c10d Stefan Wiehler    2024-10-10  1983  	mrt = ip6mr_get_table(net, raw6_sk(sk)->ip6mr_table ? : RT6_TABLE_DFLT);
9135a3aca0c10d Stefan Wiehler    2024-10-10  1984  	if (!mrt) {
9135a3aca0c10d Stefan Wiehler    2024-10-10  1985  		err = -ENOENT;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1986  		goto out;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1987  	}
9135a3aca0c10d Stefan Wiehler    2024-10-10  1988  
9135a3aca0c10d Stefan Wiehler    2024-10-10  1989  	switch (cmd) {
9135a3aca0c10d Stefan Wiehler    2024-10-10  1990  	case SIOCGETMIFCNT_IN6:
9135a3aca0c10d Stefan Wiehler    2024-10-10  1991  		if (vr.mifi >= mrt->maxvif) {
9135a3aca0c10d Stefan Wiehler    2024-10-10  1992  			err = -EINVAL;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1993  			goto out;
9135a3aca0c10d Stefan Wiehler    2024-10-10  1994  		}
9135a3aca0c10d Stefan Wiehler    2024-10-10  1995  		vr.mifi = array_index_nospec(vr.mifi, mrt->maxvif);
b70432f7319eb7 Yuval Mintz       2018-02-28  1996  		vif = &mrt->vif_table[vr.mifi];
b70432f7319eb7 Yuval Mintz       2018-02-28  1997  		if (VIF_EXISTS(mrt, vr.mifi)) {
638cf4a24a09d1 Eric Dumazet      2022-06-23  1998  			vr.icount = READ_ONCE(vif->pkt_in);
638cf4a24a09d1 Eric Dumazet      2022-06-23  1999  			vr.ocount = READ_ONCE(vif->pkt_out);
638cf4a24a09d1 Eric Dumazet      2022-06-23  2000  			vr.ibytes = READ_ONCE(vif->bytes_in);
638cf4a24a09d1 Eric Dumazet      2022-06-23  2001  			vr.obytes = READ_ONCE(vif->bytes_out);
638cf4a24a09d1 Eric Dumazet      2022-06-23  2002  			rcu_read_unlock();
e2d57766e6744f David S. Miller   2011-02-03  2003  
e2d57766e6744f David S. Miller   2011-02-03  2004  			if (copy_to_user(arg, &vr, sizeof(vr)))
e2d57766e6744f David S. Miller   2011-02-03  2005  				return -EFAULT;
e2d57766e6744f David S. Miller   2011-02-03  2006  			return 0;
e2d57766e6744f David S. Miller   2011-02-03  2007  		}
638cf4a24a09d1 Eric Dumazet      2022-06-23  2008  		rcu_read_unlock();
9135a3aca0c10d Stefan Wiehler    2024-10-10  2009  		err = -EADDRNOTAVAIL;
9135a3aca0c10d Stefan Wiehler    2024-10-10  2010  		goto out;
e2d57766e6744f David S. Miller   2011-02-03  2011  	case SIOCGETSGCNT_IN6:
e2d57766e6744f David S. Miller   2011-02-03  2012  		c = ip6mr_cache_find(mrt, &sr.src.sin6_addr, &sr.grp.sin6_addr);
e2d57766e6744f David S. Miller   2011-02-03  2013  		if (c) {
494fff56379c4a Yuval Mintz       2018-02-28  2014  			sr.pktcnt = c->_c.mfc_un.res.pkt;
494fff56379c4a Yuval Mintz       2018-02-28  2015  			sr.bytecnt = c->_c.mfc_un.res.bytes;
494fff56379c4a Yuval Mintz       2018-02-28  2016  			sr.wrong_if = c->_c.mfc_un.res.wrong_if;
87c418bf1323d5 Yuval Mintz       2018-02-28  2017  			rcu_read_unlock();
e2d57766e6744f David S. Miller   2011-02-03  2018  
e2d57766e6744f David S. Miller   2011-02-03  2019  			if (copy_to_user(arg, &sr, sizeof(sr)))
e2d57766e6744f David S. Miller   2011-02-03  2020  				return -EFAULT;
e2d57766e6744f David S. Miller   2011-02-03  2021  			return 0;
e2d57766e6744f David S. Miller   2011-02-03  2022  		}
9135a3aca0c10d Stefan Wiehler    2024-10-10  2023  		err = -EADDRNOTAVAIL;
9135a3aca0c10d Stefan Wiehler    2024-10-10  2024  		goto out;
e2d57766e6744f David S. Miller   2011-02-03  2025  	}
9135a3aca0c10d Stefan Wiehler    2024-10-10  2026  
9135a3aca0c10d Stefan Wiehler    2024-10-10  2027  out:
9135a3aca0c10d Stefan Wiehler    2024-10-10  2028  	rcu_read_unlock();
9135a3aca0c10d Stefan Wiehler    2024-10-10  2029  	return err;
e2d57766e6744f David S. Miller   2011-02-03  2030  }
e2d57766e6744f David S. Miller   2011-02-03  2031  #endif
7bc570c8b4f75d YOSHIFUJI Hideaki 2008-04-03  2032
Stefan Wiehler Oct. 14, 2024, 3:05 p.m. UTC | #5 
>>>> When IPV6_MROUTE_MULTIPLE_TABLES is enabled, calls to ip6mr_get_table()
>>>> must be done under RCU or RTNL lock. Copy from user space must be
>>>> performed beforehand as we are not allowed to sleep under RCU lock.
>>>>
>>>> Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
>>>> Fixes: d1db275dd3f6 ("ipv6: ip6mr: support multiple tables")
> 
> ...
> 
>>>> @@ -2004,11 +2020,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;
>>>>       }
>>>> +
>>>
>>> I think that this out label should be used consistently once rcu_read_lock
>>> has been taken. With this patch applied there seems to be one case on error
>>> where rcu_read_unlock() before returning, and one case where it isn't
>>> (which looks like it leaks the lock).
>>
>> In the remaining two return paths we need to release the RCU lock before
>> calling copy_to_user(), so unfortunately we cannot use a common out label.
> 
> Ok, sure. But can you check that the code always releases the lock?

Yes, it should release the lock in all cases.

Kind regards,

Stefan
diff mbox series

Patch

diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index b18eb4ad21e4..415ba6f55a44 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1961,10 +1961,7 @@  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:
@@ -1972,8 +1969,30 @@  int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
 			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);
@@ -1987,12 +2006,9 @@  int ip6mr_compat_ioctl(struct sock *sk, unsigned int cmd, void __user *arg)
 			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;
@@ -2004,11 +2020,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