Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1722735 > unrolled thread

[PATCH] driver core: bus: Fix a potential double free

Back to article view | Back to linux.kernel

Contents

  [PATCH] driver core: bus: Fix a potential double free Christophe JAILLET <christophe.jaillet@wanadoo.fr> - 2017-08-29 21:30 +0200
    Re: [PATCH] driver core: bus: Fix a potential double free Greg KH <gregkh@linuxfoundation.org> - 2017-08-31 19:00 +0200

#1722735 — [PATCH] driver core: bus: Fix a potential double free

The .release function of driver_ktype is 'driver_release()'.
This function frees the container_of this kobject.

So, this memory must not be freed explicitly in the error handling path of
'bus_add_driver()'. Otherwise a double free will occur.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
I have left the 'drv->p = NULL' even if it looks useless to me. It does not
hurt. I have also left a comment to explain why we do not free 'drv->p'
explicitly before zeroing this pointer.
---
 drivers/base/bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index e162c9a789ba..22a64fd3309b 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -698,7 +698,7 @@ int bus_add_driver(struct device_driver *drv)
 
 out_unregister:
 	kobject_put(&priv->kobj);
-	kfree(drv->p);
+	/* drv->p is freed in driver_release()  */
 	drv->p = NULL;
 out_put_bus:
 	bus_put(bus);
-- 
2.11.0

[toc] | [next] | [standalone]

#1724419

On Tue, Aug 29, 2017 at 09:23:49PM +0200, Christophe JAILLET wrote:
> The .release function of driver_ktype is 'driver_release()'.
> This function frees the container_of this kobject.
> 
> So, this memory must not be freed explicitly in the error handling path of
> 'bus_add_driver()'. Otherwise a double free will occur.

Oh, nice catch!

> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> I have left the 'drv->p = NULL' even if it looks useless to me. It does not
> hurt. I have also left a comment to explain why we do not free 'drv->p'
> explicitly before zeroing this pointer.

We need that for when people do static structures.  Like as is done for
drivers.  Well, I think that's why we need it, can't really remember,
might as well be safe, the error handling paths here are crazy at
times...

thnaks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web