All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] staging: greybus: timesync: validate platform state callback
@ 2017-01-23 16:32 Rui Miguel Silva
  2017-01-23 16:35 ` Johan Hovold
  2017-01-24  3:17 ` [greybus-dev] " Viresh Kumar
  0 siblings, 2 replies; 3+ messages in thread
From: Rui Miguel Silva @ 2017-01-23 16:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Bryan O'Donoghue
  Cc: Johan Hovold, Alex Elder, devel, greybus-dev, Rui Miguel Silva,
	# 4 . 9 . x

When tearingdown timesync, and not in arche platform, the state platform
callback is not initialized. That will trigger the following NULL
dereferencing.
CallTrace:

 ? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
 gb_timesync_teardown+0x85/0xc0 [greybus]
 gb_timesync_svc_remove+0xab/0x190 [greybus]
 gb_svc_del+0x29/0x110 [greybus]
 gb_hd_del+0x14/0x20 [greybus]
 ap_disconnect+0x24/0x60 [gb_es2]
 usb_unbind_interface+0x7a/0x2c0
 __device_release_driver+0x96/0x150
 device_release_driver+0x1e/0x30
 bus_remove_device+0xe7/0x130
 device_del+0x116/0x230
 usb_disable_device+0x97/0x1f0
 usb_disconnect+0x80/0x260
 hub_event+0x5ca/0x10e0
 process_one_work+0x126/0x3b0
 worker_thread+0x55/0x4c0
 ? process_one_work+0x3b0/0x3b0
 kthread+0xc4/0xe0
 ? kthread_park+0xb0/0xb0
 ret_from_fork+0x22/0x30

So, fix that by adding checks before use the callback.

Fixes: 970dc85bd95d ("greybus: timesync: Add timesync core driver")
Cc: <stable@vger.kernel.org> # 4.9.x
Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
---
Greg,
This patch does not contain the "commit <sha> upstream" snippet since you
already removed the timesync code from the -next tree, even though it is still
in 4.10-rc. However this will affect the greybus users of 4.9.x. If you
meanwhile pull that remove to rc, this patch is only necessary in stable 4.9.x.

v1->v2:
   - add Fixes tag to changelog
   Johan Hovold:
      - for symmetry with the lock, adjust the unlock_bus function accordantly.

 drivers/staging/greybus/timesync_platform.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/greybus/timesync_platform.c b/drivers/staging/greybus/timesync_platform.c
index 113f3d6c4b3a..27f75b17679b 100644
--- a/drivers/staging/greybus/timesync_platform.c
+++ b/drivers/staging/greybus/timesync_platform.c
@@ -45,12 +45,18 @@ u32 gb_timesync_platform_get_clock_rate(void)
 
 int gb_timesync_platform_lock_bus(struct gb_timesync_svc *pdata)
 {
+	if (!arche_platform_change_state_cb)
+		return 0;
+
 	return arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_TIME_SYNC,
 					      pdata);
 }
 
 void gb_timesync_platform_unlock_bus(void)
 {
+	if (!arche_platform_change_state_cb)
+		return;
+
 	arche_platform_change_state_cb(ARCHE_PLATFORM_STATE_ACTIVE, NULL);
 }
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] staging: greybus: timesync: validate platform state callback
  2017-01-23 16:32 [PATCH v2] staging: greybus: timesync: validate platform state callback Rui Miguel Silva
@ 2017-01-23 16:35 ` Johan Hovold
  2017-01-24  3:17 ` [greybus-dev] " Viresh Kumar
  1 sibling, 0 replies; 3+ messages in thread
From: Johan Hovold @ 2017-01-23 16:35 UTC (permalink / raw)
  To: Rui Miguel Silva
  Cc: Greg Kroah-Hartman, Bryan O'Donoghue, Johan Hovold,
	Alex Elder, devel, greybus-dev, # 4 . 9 . x

On Mon, Jan 23, 2017 at 04:32:57PM +0000, Rui Miguel Silva wrote:
> When tearingdown timesync, and not in arche platform, the state platform
> callback is not initialized. That will trigger the following NULL
> dereferencing.
> CallTrace:
> 
>  ? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
>  gb_timesync_teardown+0x85/0xc0 [greybus]
>  gb_timesync_svc_remove+0xab/0x190 [greybus]
>  gb_svc_del+0x29/0x110 [greybus]
>  gb_hd_del+0x14/0x20 [greybus]
>  ap_disconnect+0x24/0x60 [gb_es2]
>  usb_unbind_interface+0x7a/0x2c0
>  __device_release_driver+0x96/0x150
>  device_release_driver+0x1e/0x30
>  bus_remove_device+0xe7/0x130
>  device_del+0x116/0x230
>  usb_disable_device+0x97/0x1f0
>  usb_disconnect+0x80/0x260
>  hub_event+0x5ca/0x10e0
>  process_one_work+0x126/0x3b0
>  worker_thread+0x55/0x4c0
>  ? process_one_work+0x3b0/0x3b0
>  kthread+0xc4/0xe0
>  ? kthread_park+0xb0/0xb0
>  ret_from_fork+0x22/0x30
> 
> So, fix that by adding checks before use the callback.
> 
> Fixes: 970dc85bd95d ("greybus: timesync: Add timesync core driver")
> Cc: <stable@vger.kernel.org> # 4.9.x
> Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>

Reviewed-by: Johan Hovold <johan@kernel.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [greybus-dev] [PATCH v2] staging: greybus: timesync: validate platform state callback
  2017-01-23 16:32 [PATCH v2] staging: greybus: timesync: validate platform state callback Rui Miguel Silva
  2017-01-23 16:35 ` Johan Hovold
@ 2017-01-24  3:17 ` Viresh Kumar
  1 sibling, 0 replies; 3+ messages in thread
From: Viresh Kumar @ 2017-01-24  3:17 UTC (permalink / raw)
  To: Rui Miguel Silva
  Cc: Greg Kroah-Hartman, Bryan O'Donoghue, devel, Johan Hovold,
	# 4 . 9 . x, greybus-dev, Alex Elder

On 23-01-17, 16:32, Rui Miguel Silva wrote:
> When tearingdown timesync, and not in arche platform, the state platform
> callback is not initialized. That will trigger the following NULL
> dereferencing.
> CallTrace:
> 
>  ? gb_timesync_platform_unlock_bus+0x11/0x20 [greybus]
>  gb_timesync_teardown+0x85/0xc0 [greybus]
>  gb_timesync_svc_remove+0xab/0x190 [greybus]
>  gb_svc_del+0x29/0x110 [greybus]
>  gb_hd_del+0x14/0x20 [greybus]
>  ap_disconnect+0x24/0x60 [gb_es2]
>  usb_unbind_interface+0x7a/0x2c0
>  __device_release_driver+0x96/0x150
>  device_release_driver+0x1e/0x30
>  bus_remove_device+0xe7/0x130
>  device_del+0x116/0x230
>  usb_disable_device+0x97/0x1f0
>  usb_disconnect+0x80/0x260
>  hub_event+0x5ca/0x10e0
>  process_one_work+0x126/0x3b0
>  worker_thread+0x55/0x4c0
>  ? process_one_work+0x3b0/0x3b0
>  kthread+0xc4/0xe0
>  ? kthread_park+0xb0/0xb0
>  ret_from_fork+0x22/0x30
> 
> So, fix that by adding checks before use the callback.
> 
> Fixes: 970dc85bd95d ("greybus: timesync: Add timesync core driver")
> Cc: <stable@vger.kernel.org> # 4.9.x
> Signed-off-by: Rui Miguel Silva <rmfrfs@gmail.com>
> ---
> Greg,
> This patch does not contain the "commit <sha> upstream" snippet since you
> already removed the timesync code from the -next tree, even though it is still
> in 4.10-rc. However this will affect the greybus users of 4.9.x. If you
> meanwhile pull that remove to rc, this patch is only necessary in stable 4.9.x.
> 
> v1->v2:
>    - add Fixes tag to changelog
>    Johan Hovold:
>       - for symmetry with the lock, adjust the unlock_bus function accordantly.

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-01-24  3:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-23 16:32 [PATCH v2] staging: greybus: timesync: validate platform state callback Rui Miguel Silva
2017-01-23 16:35 ` Johan Hovold
2017-01-24  3:17 ` [greybus-dev] " Viresh Kumar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.