Skip to contentSkip to conversation

staging: greybus: timesync: validate platform state callback

PATCH In review Rui Miguel Silva 3 participants 3 messages Jan 23, 2017 – Jan 24, 2017

Subsystems (3)
Stable branch Staging Greybus
lore
Share & tools
lore

Rui Miguel Silva

author
#0 lore raw

PATCH v2 1/1 staging: greybus: timesync: validate platform state callback

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 <[email protected]> # 4.9.x
Signed-off-by Rui Miguel Silva <[email protected]>

---
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(+)
drivers/staging/greybus/timesync_platform.c +6 -0
index 113f3d6c4b3a..27f75b17679b 100644
@@ -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);
 }
Signature · 1 line
2.11.0
maintainer Reviewed-by on v2 1/1
#1 lore raw
Quoted 26 lines · Rui Miguel Silva

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
Quoted 5 lines

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

Fixes 970dc85bd95d ("greybus: timesync: Add timesync core driver")
Cc <[email protected]> # 4.9.x
Signed-off-by Rui Miguel Silva <[email protected]>
Reviewed-by Johan Hovold <[email protected]>
Reviewed-by on v2 1/1
#2 lore raw
In reply to Rui Miguel Silva
Quoted 38 lines · Rui Miguel Silva

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 <[email protected]> # 4.9.x
Signed-off-by Rui Miguel Silva <[email protected]>

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

Quoted 4 lines

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

Reviewed-by Viresh Kumar <[email protected]>
Signature · 1 line
viresh