linux-kernel - Re: [PATCH] mm: mmap: Check all failures before set values

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>] [day] [month] [year] [list]

Message-ID: <20150825113521.GA6285@dhcp22.suse.cz>
Date:	Tue, 25 Aug 2015 13:35:21 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	Chen Gang <gang.chen.5i5j@...com>
Cc:	Chen Gang <xili_gchen_5257@...mail.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
	"riel@...hat.com" <riel@...hat.com>,
	"sasha.levin@...cle.com" <sasha.levin@...cle.com>,
	"gang.chen.5i5j@...il.com" <gang.chen.5i5j@...il.com>,
	Linux Memory <linux-mm@...ck.org>,
	kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm: mmap: Check all failures before set values

On Tue 25-08-15 05:54:00, Chen Gang wrote:
> On 8/24/15 21:57, Michal Hocko wrote:
> > On Mon 24-08-15 21:34:25, Chen Gang wrote:
> 
> [...]
> 
> 
> >> It is always a little better to let the external function suppose fewer
> >> callers' behalf.
> > 
> > I am sorry but I do not understand what you are saying here.
> > 
> 
> Execuse me, my English maybe be still not quite well, my meaning is:
> 
>  - For the external functions (e.g. insert_vm_struct in our case), as a
>    callee, it may have to supose something from the caller.
> 
>  - If we can keep callee's functional contents no touch, a little fewer
>    supposing will let callee a little more independent from caller.
> 
>  - If can keep functional contens no touch, the lower dependency between
>    caller and callee is always better.

OK, I guess I understand what you mean. You are certainly right that a
partial initialization for the failure case is not nice in general. I
was just objecting that the callers are supposed to free the vma in
the failure case so any partial initialization doesn't matter in this
particular case.

Your patch would be more sensible if the failure case was more
likely. But this function is used for special mappings (vdso, temporary
vdso stack) which are created early in the process life time so both
failure paths are highly unlikely. If this was a part of a larger
changes where the function would be used elsewhere I wouldn't object at
all.

The reason I am skeptical about such changes in general is that
the effect is very marginal while it increases chances of the code
conflicts.

But as I've said, if others feel this is worthwhile I will not object.

-- 
Michal Hocko
SUSE Labs
--
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/