[1/2] vznetstat: Add protection to venet_acct_set_classes()

Submitted by Kirill Tkhai on Dec. 19, 2017, 12:24 p.m.

Details

Message ID 151368624337.27901.15825540514799719437.stgit@localhost.localdomain
State New
Series "Series without cover letter"
Headers show

Commit Message

Kirill Tkhai Dec. 19, 2017, 12:24 p.m.
It seems there was no synchronization since the time
when ioctls in kernel were serialized via single mutex.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 kernel/ve/vznetstat/vznetstat.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/kernel/ve/vznetstat/vznetstat.c b/kernel/ve/vznetstat/vznetstat.c
index 3a53ce27bde2..a65e05378ff4 100644
--- a/kernel/ve/vznetstat/vznetstat.c
+++ b/kernel/ve/vznetstat/vznetstat.c
@@ -52,6 +52,7 @@  static struct class_info_set *info_v4 = NULL;
 #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
 static struct class_info_set *info_v6 = NULL;
 #endif
+static DEFINE_MUTEX(info_mutex);
 
 /* v6: flag IPv6 classes or IPv4 */
 static int venet_acct_set_classes(const void __user *user_info, int length, int v6)
@@ -88,15 +89,17 @@  static int venet_acct_set_classes(const void __user *user_info, int length, int
 			goto out_free;
 	}
 
-	rcu_read_lock();
+	mutex_lock(&info_mutex);
 	if (v6) {
-		old = rcu_dereference(info_v6);
+		old = rcu_dereference_protected(info_v6,
+						lockdep_is_held(&info_mutex));
 		rcu_assign_pointer(info_v6, info);
 	} else {
-		old = rcu_dereference(info_v4);
+		old = rcu_dereference_protected(info_v4,
+						lockdep_is_held(&info_mutex));
 		rcu_assign_pointer(info_v4, info);
 	}
-	rcu_read_unlock();
+	mutex_unlock(&info_mutex);
 
 	synchronize_net();
 	/* IMPORTANT. I think reset of statistics collected should not be

Comments

Andrey Ryabinin Dec. 20, 2017, 8:53 a.m.
On 12/19/2017 03:24 PM, Kirill Tkhai wrote:
> It seems there was no synchronization since the time
> when ioctls in kernel were serialized via single mutex.
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  kernel/ve/vznetstat/vznetstat.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/ve/vznetstat/vznetstat.c b/kernel/ve/vznetstat/vznetstat.c
> index 3a53ce27bde2..a65e05378ff4 100644
> --- a/kernel/ve/vznetstat/vznetstat.c
> +++ b/kernel/ve/vznetstat/vznetstat.c
> @@ -52,6 +52,7 @@ static struct class_info_set *info_v4 = NULL;
>  #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
>  static struct class_info_set *info_v6 = NULL;
>  #endif
> +static DEFINE_MUTEX(info_mutex);
>  
>  /* v6: flag IPv6 classes or IPv4 */
>  static int venet_acct_set_classes(const void __user *user_info, int length, int v6)
> @@ -88,15 +89,17 @@ static int venet_acct_set_classes(const void __user *user_info, int length, int
>  			goto out_free;
>  	}
>  
> -	rcu_read_lock();
> +	mutex_lock(&info_mutex);
>  	if (v6) {
> -		old = rcu_dereference(info_v6);
> +		old = rcu_dereference_protected(info_v6,
> +						lockdep_is_held(&info_mutex));
>  		rcu_assign_pointer(info_v6, info);
>  	} else {
> -		old = rcu_dereference(info_v4);
> +		old = rcu_dereference_protected(info_v4,
> +						lockdep_is_held(&info_mutex));
>  		rcu_assign_pointer(info_v4, info);

It probably would be easier to simply use xchg() here. Locking wouldn't be needed in that case.
But as I assume this is not performance sensitive code, so mutex should be fine.

Acked-by: Andrey Ryabinin <aryabinin@virtuozzo.com>


>  	}
> -	rcu_read_unlock();
> +	mutex_unlock(&info_mutex);
>  
>  	synchronize_net();
>  	/* IMPORTANT. I think reset of statistics collected should not be
>