From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932519AbcEXMHx (ORCPT ); Tue, 24 May 2016 08:07:53 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:36629 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751299AbcEXMHw (ORCPT ); Tue, 24 May 2016 08:07:52 -0400 From: Nicolai Stange To: Kees Cook Cc: Nicolai Stange , Greg Kroah-Hartman , Sasha Levin , Andrew Morton , Dmitry Vyukov , Andrey Ryabinin , James Morse , LKML Subject: Re: [PATCH] kernel/kcov: unproxify debugfs file's fops References: <1464011147-31836-1-git-send-email-nicstange@gmail.com> Date: Tue, 24 May 2016 14:07:47 +0200 In-Reply-To: (Kees Cook's message of "Mon, 23 May 2016 11:00:09 -0700") Message-ID: <87twhnek18.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.94 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Kees Cook writes: > On Mon, May 23, 2016 at 6:45 AM, Nicolai Stange wrote: >> Since commit 49d200deaa68 ("debugfs: prevent access to removed files' >> private data"), a debugfs file's file_operations methods get proxied >> through lifetime aware wrappers. >> >> However, only a certain subset of the file_operations members is supported >> by debugfs and ->mmap isn't among them -- it appears to be NULL from the >> VFS layer's perspective. >> >> This behaviour breaks the /sys/kernel/debug/kcov file introduced >> concurrently with commit 5c9a8750a640 ("kernel: add kcov code coverage"). >> >> Since that file never gets removed, there is no file removal race and thus, >> a lifetime checking proxy isn't needed. >> >> Avoid the proxying for /sys/kernel/debug/kcov by creating it via >> debugfs_create_file_unsafe() rather than debugfs_create_file(). >> >> Fixes: 49d200deaa68 ("debugfs: prevent access to removed files' private >> data") >> Fixes: 5c9a8750a640 ("kernel: add kcov code coverage") >> Signed-off-by: Nicolai Stange >> --- >> Applicable to linux-next 20160523. >> In particular, it depends on >> - c64688081490 ("debugfs: add support for self-protecting attribute file >> fops") >> - 5c9a8750a640 ("kernel: add kcov code coverage") >> >> This issue has been debugged and reported by >> Sasha Levin : >> http://lkml.kernel.org/g/573F4200.3080208@oracle.com >> >> kernel/kcov.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/kcov.c b/kernel/kcov.c >> index a02f2dd..4c349dd 100644 >> --- a/kernel/kcov.c >> +++ b/kernel/kcov.c >> @@ -264,7 +264,7 @@ static const struct file_operations kcov_fops = { >> >> static int __init kcov_init(void) >> { >> - if (!debugfs_create_file("kcov", 0600, NULL, NULL, &kcov_fops)) { >> + if (!debugfs_create_file_unsafe("kcov", 0600, NULL, NULL, &kcov_fops)) { > > It might make sense to add a comment above this to describe why > "unsafe" is not unsafe in this case. Done. v2 can be found at http://lkml.kernel.org/g/1464091505-20943-1-git-send-email-nicstange@gmail.com Thanks, Nicolai > > -Kees > >> pr_err("failed to create kcov in debugfs\n"); >> return -ENOMEM; >> } >> -- >> 2.8.2 >>