Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 17 Jan 2017 19:26:07 +0100
From: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To: "Reshetova, Elena" <elena.reshetova@...el.com>
Cc: Kees Cook <keescook@...omium.org>,
	Peter Zijlstra <peterz@...radead.org>,
	AKASHI Takahiro <takahiro.akashi@...aro.org>,
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
	"arnd@...db.de" <arnd@...db.de>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"Anvin, H Peter" <h.peter.anvin@...el.com>,
	"will.deacon@....com" <will.deacon@....com>,
	"dwindsor@...il.com" <dwindsor@...il.com>,
	"ishkamiel@...il.com" <ishkamiel@...il.com>
Subject: Re: [RFC PATCH 08/19] kernel, mm: convert from
 atomic_t to refcount_t

On Tue, Jan 17, 2017 at 05:44:07PM +0000, Reshetova, Elena wrote:
> > On Mon, Jan 16, 2017 at 8:16 AM, Reshetova, Elena
> > <elena.reshetova@...el.com> wrote:
> > >> On Thu, Jan 12, 2017 at 02:11:15PM +0900, AKASHI Takahiro wrote:
> > >> > On Wed, Jan 11, 2017 at 02:55:21PM -0800, Kees Cook wrote:
> > >> > > On Wed, Jan 11, 2017 at 1:42 PM, Kees Cook <keescook@...omium.org>
> > >> wrote:
> > >> > > > I can see if it'll cherry-pick cleanly, I assume it will. :)
> > >> > >
> > >> > > It cherry-picked cleanly. However, I made several changes:
> > >> > >
> > >> > > - I adjusted Peter's author email (it had extra []s around).
> > >> > > - I fixed all of the commit subjects (Peter's were missing).
> > >> > > - I added back "kref: Add KREF_INIT()" since it seems to have been
> > >> > > lost and mixed into other patches that would break bisection
> > >> > >
> > >> > > It's here now, please work from this version:
> > >> > >
> > >> > >
> > >>
> > http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=kspp/hardened-
> > >> atomic
> > >> >
> > >> > I gave it a spin on arm64.
> > >> > It can compile with a change to smp.c that I mentioned before,
> > >> > but the boot failed. I've not dug into it.
> > >> >
> > >> > ===8<===
> > >> > [    3.578618] refcount_t: increment on 0; use-after-free.
> > >> > [    3.579165] ------------[ cut here ]------------
> > >> > [    3.579254] WARNING: CPU: 0 PID: 1 at
> > /home/akashi/arm/armv8/linaro/linux-
> > >> aarch64/include/linux/refcount.h:109 unx_create+0x8c/0xc0
> > >>
> > >> That's dodgy code, someone needs to look at that.
> > >>
> > >> It has an inc in a function called 'create' which seems to suggest its
> > >> objection creation and we should be using refcount_set() instead.
> > >>
> > >> Then again, it looks like you can call this 'create' method multiple
> > >> times, each time returning the same static object, so refcount_set()
> > >> would not be correct.
> > >>
> > >> Using a refcount on a static object is weird of course, so this is bound
> > >> to give trouble.
> > >
> > > I have reverted this one back to atomic and added it to the tracking doc.
> > > The problem for this one is that it is not always used as static and in other cases
> > > it is even initialized correctly to 1, but this static case seems to be special one
> > giving troubles...
> > >
> > > Last week I also fixed all the warnings/errors that test infra gave. The question
> > that comes is what next? How do we really test this further apart from just booting
> > this up?
> > 
> > Which tree has all the fixes? I can refresh my kernel.org tree and let
> > 0day grind on it, then we can start getting acks and I can push it
> > into -next via my KSPP tree.
> 
> Here is the tree: https://github.com/ereshetova/linux-stable/commits/refcount_t
> 
> I would really like to get more runtime testing done for it also, not just asks :)

Can you post the fixed up patches so that we can properly review them?

thanks,

greg k-h

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.