linux-kernel - Re: [PATCH] ACPI: Do not fail acpi_bind_one() if device is already bound correctly

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3596381.TNBAQz9C67@vostro.rjw.lan>
Date:	Sat, 03 Aug 2013 02:47:04 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Toshi Kani <toshi.kani@...com>
Cc:	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH] ACPI: Do not fail acpi_bind_one() if device is already bound correctly

On Friday, August 02, 2013 04:38:38 PM Toshi Kani wrote:
> On Fri, 2013-08-02 at 00:33 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > 
> > Modify acpi_bind_one() so that it doesn't fail if the device
> > represented by its first argument has already been bound to the
> > given ACPI handle (second argument), because that is not a good
> > enough reason for returning an error code.
> 
> While it seems reasonable to allow such case, I do not think we will hit
> this case under the normal scenarios.  So, I do not think we need to
> make this change now unless it actually solves Yasuaki's issue (which I
> am guessing not).

In theory it should be possible to call acpi_bind_one() twice in a row
for the same dev and the same handle without failure, that simply is
logical.  The patch may not fix any problems visible now, but returning an
error code in such a case is simply incorrect.

Thanks,
Rafael


> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > ---
> >  drivers/acpi/glue.c |   15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > Index: linux-pm/drivers/acpi/glue.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/glue.c
> > +++ linux-pm/drivers/acpi/glue.c
> > @@ -143,7 +143,10 @@ int acpi_bind_one(struct device *dev, ac
> >  	list_for_each_entry(pn, &acpi_dev->physical_node_list, node)
> >  		if (pn->dev == dev) {
> >  			dev_warn(dev, "Already associated with ACPI node\n");
> > -			goto err_free;
> > +			if (ACPI_HANDLE(dev) == handle)
> > +				retval = 0;
> > +
> > +			goto out_free;
> >  		}
> >  
> >  	/* allocate physical node id according to physical_node_id_bitmap */
> > @@ -152,7 +155,7 @@ int acpi_bind_one(struct device *dev, ac
> >  		ACPI_MAX_PHYSICAL_NODE);
> >  	if (physical_node->node_id >= ACPI_MAX_PHYSICAL_NODE) {
> >  		retval = -ENOSPC;
> > -		goto err_free;
> > +		goto out_free;
> >  	}
> >  
> >  	set_bit(physical_node->node_id, acpi_dev->physical_node_id_bitmap);
> > @@ -185,10 +188,14 @@ int acpi_bind_one(struct device *dev, ac
> >  	put_device(dev);
> >  	return retval;
> >  
> > - err_free:
> > + out_free:
> >  	mutex_unlock(&acpi_dev->physical_node_lock);
> >  	kfree(physical_node);
> > -	goto err;
> > +	if (retval)
> > +		goto err;
> > +
> > +	put_device(dev);
> > +	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(acpi_bind_one);
> >  
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/