Re: [PATCH] kernel: user_namespace: always set the return parameter 'new_cred' when call unshare_userns() successfully.

[Chen Gang <gang.chen@asianux.com>]

On 08/21/2013 07:57 PM, Oleg Nesterov wrote:
> On 08/21, Chen Gang wrote:
>>
>> On 08/20/2013 10:37 PM, Oleg Nesterov wrote:
>>> On 08/20, Serge Hallyn wrote:
>>>>
>>>>
>>>> But the only existing caller (sys_unshare) does in fact initialize it to
>>>> NULL.  So while this patch does no harm, is it necessary?
>>>
>>> Agreed.
>>>
>>> Plus, with this patch unshare_userns() becomes "inconsistent" compared
>>> to other unshare_ helpers.
>>>
>>
>> Hmm... for static functions, they don't need, but for extern functions,
>> recommend to do so.
>>
>>
>> For "unshare_ helpers", I find 3 extern functions:
>>
>>   unshare_files() which already set value.
>>   unshare_userns() and unshare_nsproxy_namespaces() which not set.
>>
>> In my opinion, recommend to always set the return parameter when
>> succeed, for the 2 left extern functions.
> 
> I do not think that static/extern should make any difference. To me,
> the cleanup should make unshare_userns() return "cred *" (or NULL or
> ERR_PTR()) But this is subjective and in this case we should change
> other unshare_ helpers too. Doesn't worth the trouble.
> 

Normal static function (excluding callback function) is used inside, it
doesn't belong to interface, but extern function belongs to interface
(it is used by outside which may be out of control by inside).

For interface, need implement its definitions precisely, in our case,
when return succeed, if 'new_cred' should be NULL, inside need set it
explicitly to be sure of it to outside.

And for the other extern "unshare_ helpers", recommend to do so, too.


Currently, it is really not a bug, but in the future, it may will be.


> Oleg.
> 
> 
> 

Thanks.
-- 
Chen Gang