linux-kernel - Re: [PATCH 02/33] mm: overload get_user

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

Message-ID: <20160216083606.GB3335@gmail.com>
Date:	Tue, 16 Feb 2016 09:36:06 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Dave Hansen <dave@...1.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org,
	torvalds@...ux-foundation.org, dave.hansen@...ux.intel.com,
	srikar@...ux.vnet.ibm.com, vbabka@...e.cz,
	akpm@...ux-foundation.org, kirill.shutemov@...ux.intel.com,
	aarcange@...hat.com, n-horiguchi@...jp.nec.com, jack@...e.cz
Subject: Re: [PATCH 02/33] mm: overload get_user_pages() functions

* Dave Hansen <dave@...1.net> wrote:

> 
> From: Dave Hansen <dave.hansen@...ux.intel.com>
> 
> The concept here was a suggestion from Ingo.  The implementation
> horrors are all mine.
> 
> This allows get_user_pages(), get_user_pages_unlocked(), and
> get_user_pages_locked() to be called with or without the
> leading tsk/mm arguments.  We will give a compile-time warning
> about the old style being __deprecated and we will also
> WARN_ON() if the non-remote version is used for a remote-style
> access.

So at minimum this should be WARN_ON_ONCE(), to make it easier to recover some 
meaningful kernel log from such incidents.

But:

> Doing this, folks will get nice warnings and will not break the
> build.  This should be nice for -next and will hopefully let
> developers fix up their own code instead of maintainers needing
> to do it at merge time.
> 
> The way we do this is hideous.  It uses the __VA_ARGS__ macro
> functionality to call different functions based on the number
> of arguments passed to the macro.
> 
> There's an additional hack to ensure that our EXPORT_SYMBOL()
> of the deprecated symbols doesn't trigger a warning.
> 
> We should be able to remove this mess as soon as -rc1 hits in
> the release after this is merged.

So when I suggested this then it looked a _lot_ cleanear to me, in my head!

OTOH this, if factored out a bit perhaps, could be the basis for a useful 
technical model to do 'phased in, -next invariant' prototype migrations in the 
future, especially when it involves lots of subsystems.

Strictly only in cases where -rc1 will truly get rid of the __VA_ARGS__ hackery - 
which we'd do in this case.

Nevertheless I'd love to have a high level buy-in from either Linus or Andrew that 
we can do it this way, as the hackery looks very hideous...

The alternative would be to allow the -next churn and to allow the occasional 
(fairly trivial but tester-disruptive) build breakage.

Thanks,

	Ingo