From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752457AbcFUQzh (ORCPT <rfc822;w@1wt.eu>);
	Tue, 21 Jun 2016 12:55:37 -0400
Received: from plane.gmane.org ([80.91.229.3]:52525 "EHLO plane.gmane.org"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751804AbcFUQzc (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Tue, 21 Jun 2016 12:55:32 -0400
X-Injected-Via-Gmane: http://gmane.org/
To: linux-kernel@vger.kernel.org
From: Jason Hrycay <jason.hrycay@motorola.com>
Subject: Re: [PATCH] driver core: fix race between creating/querying glue dir and its cleanup
Date: Tue, 21 Jun 2016 15:46:16 +0000 (UTC)
Message-ID: <loom.20160621T174448-519@post.gmane.org>
References: <1459153631-10472-1-git-send-email-ming.lei@canonical.com> <571FA500.2080500@codeaurora.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
X-Complaints-To: usenet@ger.gmane.org
X-Gmane-NNTP-Posting-Host: sea.gmane.org
User-Agent: Loom/3.14 (http://gmane.org/)
X-Loom-IP: 144.188.128.1 (Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Ming/Chandrasekhar,

Chandra Sekhar Lingutla <clingutla <at> codeaurora.org> writes:

> 
> Hi Ming,
> 
> [...]
> > +static inline bool live_in_glue_dir(struct kobject *kobj,
> > +				    struct device *dev)
> > +{
> > +	if (!kobj || !dev->class ||
> > +	    kobj->kset != &dev->class->p->glue_dirs)
> > +		return true;
> > +	return false;
> > +}
> I think we should return false if kobj->kset != &dev->class->p->glue_dirs.
> If kboj->kset points to dev->class->p->glue_dirs, then we live in glue dir.
> So logic should be:
> 	if (!kobj || !dev->class ||
> 		kobj->kset != &dev->class->p->glue_dirs)
> 			return false;
> 	return true;
> 
> > +
> > +static inline struct kobject *get_glue_dir(struct device *dev)
> > +{
> > +	if (live_in_glue_dir(&dev->kobj, dev))
> > +		return dev->kobj.parent;
> > +	return NULL;
> > +}

I don't think we should be checking the live_in_glue_dir on dev->kobj above, 
but rather, dev->kobj.parent. That being said, I don't think the check is 
even needed as it's going to be re-checked in the cleanup_glue_dir.

The issue is, if we fail the 'live_in_glue_dir' check on the dev->kobj, we'll
return NULL and subsequently fail to kobject_put the dev->kobj.parent in the 
cleanup_glue_dir function, leaking a reference.

> [snip]

Regards,
Jason Hrycay
jason.hrycay@motorola.com