Re: [PATCH v6 2/8] debugfs: prevent access to removed files' private data

From: Nicolai Stange <nicstange@gmail.com>
To: Sasha Levin <sasha.levin@oracle.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Nicolai Stange <nicstange@gmail.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Jonathan Corbet <corbet@lwn.net>, Jan Kara <jack@suse.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Julia Lawall <Julia.Lawall@lip6.fr>,
	Gilles Muller <Gilles.Muller@lip6.fr>,
	Nicolas Palix <nicolas.palix@imag.fr>,
	Michal Marek <mmarek@suse.com>,
	linux-kernel@vger.kernel.org, cocci@systeme.lip6.fr
Subject: Re: [PATCH v6 2/8] debugfs: prevent access to removed files' private data
Date: Sat, 21 May 2016 19:57:38 +0200	[thread overview]
Message-ID: <874m9rcmzx.fsf@gmail.com> (raw)
In-Reply-To: <573F4200.3080208@oracle.com> (Sasha Levin's message of "Fri, 20 May 2016 12:57:36 -0400")

Sasha Levin <sasha.levin@oracle.com> writes:

> On 05/18/2016 12:05 PM, Greg Kroah-Hartman wrote:
>> On Wed, May 18, 2016 at 11:18:16AM -0400, Sasha Levin wrote:
>>> On 05/18/2016 11:01 AM, Nicolai Stange wrote:
>>>> Thanks a million for reporting!
>>>>
>>>> 1.) Do you have lockdep enabled?
>>>
>>> Yup, nothing there.
>>>
>>>> 2.) Does this happen before or after userspace init has been spawned,
>>>>     i.e. does the lockup happen at debugfs file creation time or
>>>>     possibly at usage time?
>>>
>>> So I looked closer, and it seems to happen after starting syzkaller, which
>>> as far as I know tries to open many different debugfs files.
>>>
>>> Is there debug code I can add it that'll help us figure out what's up?
>> 
>> Trying to figure out _which_ debugfs file is causing this would be
>> great, if at all possible.  strace?
>
> What seems to be failing is syzkaller's attempt to mmap the coverage
> debugfs file. So this isn't actually a kernel deadlock but syzkaller
> misbehaves when that scenario happens.
>
> Either way, it only fails to mmap with that commit that I've pointed
> out.

That info is really helpful here: the proxy file_operations introduced by
this commit doesn't have a ->mmap() defined, i.e. it is NULL from the
VFS layer's point of view.

The simple reason is that at the time I submitted this series, my
Coccinelle script didn't find any debugfs user with a ->mmap()
defined. Thus either that script was broken or things have changed in
the meanwhile.

I'll look into this tomorrow.

Thank you very much for the effort you put into this!

>
> 	th->cover_fd = open("/sys/kernel/debug/kcov", O_RDWR);
> 	if (th->cover_fd == -1)
> 		fail("open of /sys/kernel/debug/kcov failed");
> 	if (ioctl(th->cover_fd, KCOV_INIT_TRACE, kCoverSize))
> 		fail("cover enable write failed");
> 	th->cover_data = (uintptr_t*)mmap(NULL, kCoverSize * sizeof(th->cover_data[0]), PROT_READ | PROT_WRITE, MAP_SHARED, th->cover_fd, 0);
> 	if ((void*)th->cover_data == MAP_FAILED)
> 		fail("cover mmap failed");
>
> And it's the mmap() that fails with -ENODEV.