[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170131070511.GB5149@kroah.com>
Date: Tue, 31 Jan 2017 08:05:11 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Marek Vasut <marex@...x.de>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
hch@....de, Pantelis Antoniou <pantelis.antoniou@...sulko.com>
Subject: Re: [PATCH] [RFC] fs: Possible filp_open race experiment
On Tue, Jan 31, 2017 at 06:29:36AM +0100, Marek Vasut wrote:
> +CC Greg, LKML as I don't quite know where this should go.
You do know about linux-fsdevel, right?
> On 01/18/2017 12:16 AM, Marek Vasut wrote:
> > I believe there is a possible race condition when configfs attributes
> > trigger filp_open() from the kernel. I initially observed the problem
> > on Linux 4.4 when loading DT overlay , which in turn loads a driver
> > which loads firmware. After some further investigation, I came up with
> > the following minimal-ish example patch, which can trigger the same
> > behavior on Linux 4.10-rc4 (next 20170117).
What in-kernel code causes this problem? I didn't think DT overlays
were a feature in 4.4, are you running with code that isn't in the
normal releases?
> > The core of the demo is in cfs_over_item_dtbo_write(), which just checks
> > for valid current->fs . This function is triggered by writing data into
> > configfs binary attribute, ie.:
Why are you caring about current->fs?
> > $ mkdir /sys/kernel/config/test/overlays/1/dtbo
> > $ cat file_17201_bytes_long > /sys/kernel/config/test/overlays/1/dtbo
> >
> > I believe the 'cat' program exits quickly and thus calls fs_exit()
> > before the cfs_over_item_dtbo_write() is called.
How can exit be called before write?
> > Any attempts to
> > access FS (like ie. loading firmware from FS) from that function will
> > therefore fail (by crashing the kernel, NULL pointer dereference in
> > set_root_rcu() in fs/namei.c).
> >
> > On the other hand, replacing 'cat' with 'dd' yields different result:
> >
> > $ dd if=file_17201_bytes_long of=/sys/kernel/config/test/overlays/1/dtbo
> >
> > The kernel does not crash. I believe this is because dd takes slightly
> > longer to complete, so the cfs_over_item_dtbo_write() can complete
> > before the dd process gets to calling fs_exit() and so the filesystem
> > access is still available, thus current->fs is valid.
cat and dd act differently, if you strace them, it should show the
differences, perhaps you can narrow it down there?
> > Note that when using DT overlays (whose configfs interface is not yet
> > mainline),
Ah, we can't do anything about code that is not merged, perhaps it is
just buggy? :)
> > there can easily be a device which requires a firmware in
> > the DT overlay. Such device will invoke firmware load, which uses the
> > filp_open() and will thus trigger the behavior above. Depending on
> > whether one uses dd or cat, the kernel will either crash or not.
> >
> > Any ideas ?
I think you need to fix your device tree overlay code...
thanks,
greg k-h
Powered by blists - more mailing lists