lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 18 May 2016 18:32:15 +0200 From: Nicolai Stange <nicstange@...il.com> To: Sasha Levin <sasha.levin@...cle.com> Cc: Nicolai Stange <nicstange@...il.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Rasmus Villemoes <linux@...musvillemoes.dk>, "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>, Alexander Viro <viro@...iv.linux.org.uk>, Jonathan Corbet <corbet@....net>, Jan Kara <jack@...e.com>, Andrew Morton <akpm@...ux-foundation.org>, Julia Lawall <Julia.Lawall@...6.fr>, Gilles Muller <Gilles.Muller@...6.fr>, Nicolas Palix <nicolas.palix@...g.fr>, Michal Marek <mmarek@...e.com>, linux-kernel@...r.kernel.org, cocci@...teme.lip6.fr Subject: Re: [PATCH v6 2/8] debugfs: prevent access to removed files' private data Sasha Levin <sasha.levin@...cle.com> writes: > 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? Could you try the patch below? I stared at the new full_proxy_open() for a while now and had to recognize the fact that if the original real_fops' ->open() fails, then its owning module's reference won't ever get dropped :( diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 6eb58a8..2e663d4 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -263,10 +263,14 @@ static int full_proxy_open(struct inode *inode, struct file *filp) if (real_fops->open) { r = real_fops->open(inode, filp); - if (filp->f_op != proxy_fops) { + if (r) { + replace_fops(filp, d_inode(dentry)->i_fop); + goto free_proxy; + } else if (filp->f_op != proxy_fops) { /* No protection against file removal anymore. */ WARN(1, "debugfs file owner replaced proxy fops: %pd", dentry); + replace_fops(filp, d_inode(dentry)->i_fop); goto free_proxy; } } I don't see directly how this could lead to lockups, but I think it's better to rule out the obvious before inserting more or less random printks... Thank you very much again, Nicolai
Powered by blists - more mailing lists