Re: [PATCH] kernel/kcov: unproxify debugfs file's fops
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.3080...@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-nicsta...@gmail.com Thanks, Nicolai > > -Kees > >> pr_err("failed to create kcov in debugfs\n"); >> return -ENOMEM; >> } >> -- >> 2.8.2 >>
Re: [PATCH] kernel/kcov: unproxify debugfs file's fops
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.3080...@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. -Kees > pr_err("failed to create kcov in debugfs\n"); > return -ENOMEM; > } > -- > 2.8.2 > -- Kees Cook Chrome OS & Brillo Security
[PATCH] kernel/kcov: unproxify debugfs file's fops
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.3080...@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)) { pr_err("failed to create kcov in debugfs\n"); return -ENOMEM; } -- 2.8.2