Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 18 Jan 2017 08:41:17 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: David Windsor <dwindsor@...il.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>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"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 12:44 PM, Reshetova, Elena
> <elena.reshetova@...el.com> 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 :)
> 
> Do you have any particular workload that you've been testing these with?

No, we only tested the full boot, that's why I would like to understand how to test more. 
I think it is not so much about the workload, but about testing different configuration. 
Like for example, when AKASHI Takahiro run the patches on top on NFS rootfs, it has shown the issue we haven't seen in our case. 
You can imagine how many of such cases are still hiding given the number of configurations and drivers that get active in runtime. 

Best Regards,
Elena.

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.