Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 7 May 2020 13:11:32 +0000 (UTC)
From:      Andriy Gapon <avg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r360779 - head/sys/dev/gpio
Message-ID:  <202005071311.047DBWqO094461@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: avg
Date: Thu May  7 13:11:32 2020
New Revision: 360779
URL: https://svnweb.freebsd.org/changeset/base/360779

Log:
  gpioiic_attach: fix a NULL pointer crash on hints-based systems
  
  The attach method uses GPIO_GET_BUS() to get a "newbus" device
  that provides a pin.  But on hints-based systems a GPIO controller
  driver might not be fully initialized yet and it does not know gpiobus
  hanging off it.  Thus, GPIO_GET_BUS() cannot be called yet.
  The reason is that controller drivers typically create a child gpiobus
  using gpiobus_attach_bus() and that leads to the following call chain:
  gpiobus_attach_bus() -> gpiobus_attach() ->
  bus_generic_attach(gpiobus) -> gpioiic_attach().
  So, gpioiic_attach() is called before gpiobus_attach_bus() returns.
  
  I observed this bug with nctgpio driver on amd64.
  I think that the problem was introduced in r355276.
  
  The fix is to avoid calling GPIO_GET_BUS() from the attach method.
  Instead, we know that on hints-based systems only the parent gpiobus can
  provide the pins.
  Nothing is changed for FDT-based systems.
  
  MFC after:	1 week

Modified:
  head/sys/dev/gpio/gpioiic.c

Modified: head/sys/dev/gpio/gpioiic.c
==============================================================================
--- head/sys/dev/gpio/gpioiic.c	Thu May  7 12:43:28 2020	(r360778)
+++ head/sys/dev/gpio/gpioiic.c	Thu May  7 13:11:32 2020	(r360779)
@@ -303,10 +303,20 @@ gpioiic_attach(device_t dev)
 		return (ENXIO);
 	}
 
-	/* Say what we came up with for pin config. */
+	/*
+	 * Say what we came up with for pin config.
+	 * NB: in the !FDT case the controller driver might not be set up enough
+	 * for GPIO_GET_BUS() to work.  Also, our parent is the only gpiobus
+	 * that can provide our pins.
+	 */
 	device_printf(dev, "SCL pin: %s:%d, SDA pin: %s:%d\n",
+#ifdef FDT
 	    device_get_nameunit(GPIO_GET_BUS(sc->sclpin->dev)), sc->sclpin->pin,
 	    device_get_nameunit(GPIO_GET_BUS(sc->sdapin->dev)), sc->sdapin->pin);
+#else
+	    device_get_nameunit(device_get_parent(dev)), sc->sclpin->pin,
+	    device_get_nameunit(device_get_parent(dev)), sc->sdapin->pin);
+#endif
 
 	/* Add the bitbang driver as our only child; it will add iicbus. */
 	device_add_child(sc->dev, "iicbb", -1);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202005071311.047DBWqO094461>