From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751072AbdAaHFl (ORCPT ); Tue, 31 Jan 2017 02:05:41 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:34500 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750887AbdAaHFb (ORCPT ); Tue, 31 Jan 2017 02:05:31 -0500 Date: Tue, 31 Jan 2017 08:05:11 +0100 From: Greg Kroah-Hartman To: Marek Vasut Cc: "linux-kernel@vger.kernel.org" , hch@lst.de, Pantelis Antoniou Subject: Re: [PATCH] [RFC] fs: Possible filp_open race experiment Message-ID: <20170131070511.GB5149@kroah.com> References: <20170117231600.10186-1-marex@denx.de> <16b3fb38-0dfc-2d3c-7cfd-681b15432fb7@denx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <16b3fb38-0dfc-2d3c-7cfd-681b15432fb7@denx.de> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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