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:     Arend Van Spriel <arend.vanspriel@broadcom.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>
 <d75d6f90-1691-57fa-ddb2-ece84f5d262f@broadcom.com>
From:   Denis Kenzior <denkenz@gmail.com>
Message-ID: <b81ed4a0-0b41-fff2-2bab-7219db2c9e35@gmail.com>
Date:   Mon, 22 Jul 2019 06:32:44 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <d75d6f90-1691-57fa-ddb2-ece84f5d262f@broadcom.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi Arend,

On 7/18/19 3:24 AM, Arend Van Spriel wrote:
> 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.
> 

Sure, makes sense.  I've made the suggested changes for v4.

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

Regards,
-Denis