linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Prateek Sood <prsood@codeaurora.org>
To: Will Deacon <will@kernel.org>, Greg KH <gregkh@linuxfoundation.org>
Cc: syzbot <syzbot+82defefbbd8527e1c2cb@syzkaller.appspotmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzkaller-bugs@googlegroups.com, viro@zeniv.linux.org.uk,
	hdanton@sina.com, akpm@linux-foundation.org
Subject: Re: WARNING: refcount bug in cdev_get
Date: Tue, 24 Dec 2019 18:29:18 +0530	[thread overview]
Message-ID: <93629f93-d20e-fa56-b021-3a90b355e6ec@codeaurora.org> (raw)
In-Reply-To: <20191219115909.GA32361@willie-the-truck>

Hi Will,

I am facing same issue while syzkaller fault injection code is causing

failure in filp->f_op->open() from chrdev_open().

I believe we need to rely on refcount as cdev_lock() is not sufficient

in this case.

Patch mentioned in 
https://groups.google.com/forum/#!original/syzkaller-bugs/PnQNxBrWv_8/X1ygj8d8DgAJ

seems good.


Please share your opinion the same.


Regards

Prateek



On 12/19/2019 5:29 PM, Will Deacon wrote:
> On Wed, Dec 18, 2019 at 07:20:26PM +0100, Greg KH wrote:
>> On Wed, Dec 18, 2019 at 05:08:55PM +0000, Will Deacon wrote:
>>> On Tue, Dec 10, 2019 at 11:44:45AM +0000, Will Deacon wrote:
>>>> On Wed, Dec 04, 2019 at 01:31:48PM +0100, Greg KH wrote:
>>>>> This code hasn't changed in 15+ years, what suddenly changed that causes
>>>>> problems here?
>>>> I suppose one thing to consider is that the refcount code is relatively new,
>>>> so it could be that the actual use-after-free is extremely rare, but we're
>>>> now seeing that it's at least potentially an issue.
>>>>
>>>> Thoughts?
>>> FWIW, I added some mdelay()s to make this race more likely, and I can now
>>> trigger it reasonably reliably. See below.
>>>
>>> --->8
>>>
>>> [   89.512353] ------------[ cut here ]------------
>>> [   89.513350] refcount_t: addition on 0; use-after-free.
>>> [   89.513977] WARNING: CPU: 2 PID: 6385 at lib/refcount.c:25 refcount_warn_saturate+0x6d/0xf0
> [...]
>
>> No hint as to _where_ you put the mdelay()?  :)
> I threw it in the release function to maximise the period where the refcount
> is 0 but the inode 'i_cdev' pointer is non-NULL. I also hacked chrdev_open()
> so that the fops->open() call appears to fail most of the time (I guess
> syzkaller uses error injection to do something similar). Nasty hack below.
>
> I'll send a patch, given that I've managed to "reproduce" this.
>
> Will
>
> --->8
>
> diff --git a/fs/char_dev.c b/fs/char_dev.c
> index 00dfe17871ac..e2e48fcd0435 100644
> --- a/fs/char_dev.c
> +++ b/fs/char_dev.c
> @@ -375,7 +375,7 @@ static int chrdev_open(struct inode *inode, struct file *filp)
>   	const struct file_operations *fops;
>   	struct cdev *p;
>   	struct cdev *new = NULL;
> -	int ret = 0;
> +	int ret = 0, first = 0;
>   
>   	spin_lock(&cdev_lock);
>   	p = inode->i_cdev;
> @@ -395,6 +395,7 @@ static int chrdev_open(struct inode *inode, struct file *filp)
>   			inode->i_cdev = p = new;
>   			list_add(&inode->i_devices, &p->list);
>   			new = NULL;
> +			first = 1;
>   		} else if (!cdev_get(p))
>   			ret = -ENXIO;
>   	} else if (!cdev_get(p))
> @@ -411,6 +412,10 @@ static int chrdev_open(struct inode *inode, struct file *filp)
>   
>   	replace_fops(filp, fops);
>   	if (filp->f_op->open) {
> +		if (first && (get_cycles() & 0x3)) {
> +			ret = -EINTR;
> +			goto out_cdev_put;
> +		}
>   		ret = filp->f_op->open(inode, filp);
>   		if (ret)
>   			goto out_cdev_put;
> @@ -594,12 +599,14 @@ void cdev_del(struct cdev *p)
>   	kobject_put(&p->kobj);
>   }
>   
> +#include <linux/delay.h>
>   
>   static void cdev_default_release(struct kobject *kobj)
>   {
>   	struct cdev *p = container_of(kobj, struct cdev, kobj);
>   	struct kobject *parent = kobj->parent;
>   
> +	mdelay(50);
>   	cdev_purge(p);
>   	kobject_put(parent);
>   }

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

      reply	other threads:[~2019-12-24 12:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-20 22:58 syzbot
2019-12-04 11:50 ` Will Deacon
2019-12-04 12:31   ` Greg KH
2019-12-10 11:44     ` Will Deacon
2019-12-18 17:08       ` Will Deacon
2019-12-18 18:20         ` Greg KH
2019-12-19 11:59           ` Will Deacon
2019-12-24 12:59             ` Prateek Sood [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=93629f93-d20e-fa56-b021-3a90b355e6ec@codeaurora.org \
    --to=prsood@codeaurora.org \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdanton@sina.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzbot+82defefbbd8527e1c2cb@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=will@kernel.org \
    --subject='Re: WARNING: refcount bug in cdev_get' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).