linux-kernel - Re: [PATCH] cciss: return 0 from driver probe function on success, not 1

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20131101140837.GO31390@beardog.cce.hp.com>
Date:	Fri, 1 Nov 2013 09:08:37 -0500
From:	scameron@...rdog.cce.hp.com
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Tomas Henzl <thenzl@...hat.com>, Jens Axboe <axboe@...nel.dk>,
	stephenmcameron@...il.com, mikem@...rdog.cce.hp.com,
	linux-kernel@...r.kernel.org, scameron@...rdog.cce.hp.com
Subject: Re: [PATCH] cciss: return 0 from driver probe function on success, not 1

On Fri, Nov 01, 2013 at 06:31:10AM -0700, Andrew Morton wrote:
> On Fri, 01 Nov 2013 14:06:45 +0100 Tomas Henzl <thenzl@...hat.com> wrote:
> 
> > The problem in kernel is that the error handling in local_pci_probe
> > and  in __pci_device_probe is different for ret values > 0,
> > so we should fix it somewhere so it is in sync.
> > The documentation states that the probe function should return zero on success
> > so what about this -
> > 
> > This would bring the handling to sync
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index 98f7b9b..200a071 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -317,8 +317,6 @@ __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
> >  		id = pci_match_device(drv, pci_dev);
> >  		if (id)
> >  			error = pci_call_probe(drv, pci_dev, id);
> > -		if (error >= 0)
> > -			error = 0;
> >  	}
> >  	return error;
> >  }
> 
> ah, there it is.
> 
> This change would turn semi-kaput drivers into kaput-kaput drivers.  It
> would be better to add a runtime warning here so those drivers get
> fixed.  Such a warning would need to reliably identify the offending
> probe function so a simple WARN_ON() wouldn't be sufficient.
> 

FWIW, I just booted up with the following change:

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 98f7b9b..ef71bb5 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -264,9 +264,16 @@ static long local_pci_probe(void *_ddi)
 	pm_runtime_get_sync(dev);
 	pci_dev->driver = pci_drv;
 	rc = pci_drv->probe(pci_dev, ddi->id);
-	if (rc) {
+	if (rc < 0) {
 		pci_dev->driver = NULL;
 		pm_runtime_put_sync(dev);
+	} else {
+		if (rc > 0) {
+			dev_warn(dev,
+				"Driver probe function returned %d, greater than 0\n", rc);
+			rc = 0;
+			
+		}
 	}
 	return rc;
 }


And,

[scameron@...alhost linux-3.12-rc6]$ dmesg | grep 'Driver probe'
[scameron@...alhost linux-3.12-rc6]$

Not that it means all that much since I don't have hardware for
the majority of drivers, obviously.

I think the above should make drivers with probe functions
returning > 0 "work" again, but with a warning.

The question would be, are there drivers which return > 0 and
by this value intend to convey that the probe function has failed?

-- steve

--
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/