linux-kernel - Re: [PATCH] kernel: user_namespace: always set the return parameter 'new_cred' when call unshare

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5215699D.9060009@asianux.com>
Date:	Thu, 22 Aug 2013 09:30:05 +0800
From:	Chen Gang <gang.chen@...anux.com>
To:	Oleg Nesterov <oleg@...hat.com>
CC:	Serge Hallyn <serge.hallyn@...ntu.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	Andy Lutomirski <luto@...capital.net>,
	"dhowells@...hat.com" <dhowells@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kernel: user_namespace: always set the return parameter
 'new_cred' when call unshare_userns() successfully.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/