Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v3 2/3] nl80211: Limit certain commands to interface owner
To:     Denis Kenzior <denkenz@gmail.com>,
        Johannes Berg <johannes@sipsolutions.net>
Cc:     linux-wireless@vger.kernel.org
References: <20190701153317.27170-1-denkenz@gmail.com>
 <20190701153317.27170-2-denkenz@gmail.com>
From:   Arend Van Spriel <arend.vanspriel@broadcom.com>
Message-ID: <d75d6f90-1691-57fa-ddb2-ece84f5d262f@broadcom.com>
Date:   Thu, 18 Jul 2019 10:24:24 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190701153317.27170-2-denkenz@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 7/1/2019 5:33 PM, Denis Kenzior wrote:
> If the wdev object has been created (via NEW_INTERFACE) with
> SOCKET_OWNER attribute set, then limit certain commands only to the
> process that created that wdev.
> 
> This can be used to make sure no other process on the system interferes
> by sending unwanted scans, action frames or any other funny business.
> 
> This patch introduces a new internal flag, and checks that flag in the
> pre_doit hook.
> 
> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
> ---
>   net/wireless/nl80211.c | 80 ++++++++++++++++++++++++++++++++----------
>   1 file changed, 61 insertions(+), 19 deletions(-)
> 
> Changes in v3:
>    - Fix minor locking mistake reported by kernel test robot
> 
> Changes in v2:
>    - None
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index ff760ba83449..ebf5eab1f9b2 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c

[snip]

>   
> -	return 0;
> +	ret = 0;

I suggest to keep the return 0 here for success path and only do the 
below for failure case (and obviously dropping '&& ret < 0'). Maybe 
rename label 'done' to 'fail' as well.

> +done:
> +	if (rtnl && ret < 0)
> +		rtnl_unlock();
> +
> +	return ret;
>   }

Regards,
Arend