From 8d41f620180f65bc65005f05cd73e6eea224375d Mon Sep 17 00:00:00 2001 From: Dustin Brown Date: Wed, 20 Feb 2019 17:01:42 -0800 Subject: [PATCH] qcacmn: Use qdf_timer shim for all kernel versions qdf_timer_t has repeatedly been identified as a problem area by Control- Flow Integrity (CFI) analysis. This is because qdf_timer_init casts the callback from a signature taking a "void *" into one which takes an "unsigned long." While this "works," it is technically undefined behavior, and CFI is correct in flagging it. Unfortunately, CFI indicates the issue is with the callbacks themselves, which is incorrect. Using unsigned long as the callback parameter has a number serious drawbacks. Most significant is the fact that pointer size is not guaranteed to be the same size as an unsigned long on platforms other than Linux. For example, this is not the case on 64 bit Windows. As QDF is supposed to be a platform abstraction, it cannot use unsigned longs for memory addresses. Instead of casting the timer callback in qdf_timer_init, use a callback shim which takes an unsigned long, and handles the appropriate conversions needed to call the actual callback function. Change-Id: Id9149169f35f619f649934310a2a673a685690f0 CRs-Fixed: 2403021 --- qdf/inc/qdf_types.h | 3 --- qdf/linux/src/i_qdf_timer.h | 28 ++++++++++++++++++---------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/qdf/inc/qdf_types.h b/qdf/inc/qdf_types.h index f94e6a72b51f..ab516795401f 100644 --- a/qdf/inc/qdf_types.h +++ b/qdf/inc/qdf_types.h @@ -265,9 +265,6 @@ typedef void (*qdf_defer_fn_t)(void *); */ typedef bool (*qdf_irqlocked_func_t)(void *); -/* Prototype of timer function */ -typedef void (*qdf_timer_func_t)(void *); - #define qdf_offsetof(type, field) offsetof(type, field) /** diff --git a/qdf/linux/src/i_qdf_timer.h b/qdf/linux/src/i_qdf_timer.h index 55cafead14b2..521e03ddb4a7 100644 --- a/qdf/linux/src/i_qdf_timer.h +++ b/qdf/linux/src/i_qdf_timer.h @@ -34,15 +34,15 @@ #include #endif +typedef void (*qdf_timer_func_t)(void *); + #define qdf_msecs_to_jiffies(msec) \ (qdf_timer_get_multiplier() * msecs_to_jiffies(msec)) struct __qdf_timer_t { struct timer_list os_timer; -#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 15, 0) qdf_timer_func_t callback; void *context; -#endif }; #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 15, 0) @@ -81,7 +81,12 @@ static inline QDF_STATUS __qdf_timer_init(struct __qdf_timer_t *timer, __setup_timer((timer), (fn), (data), TIMER_DEFERRABLE) #endif -typedef void (*__legacy_timer_callback_t)(unsigned long arg); +static inline void __os_timer_shim(unsigned long addr) +{ + struct __qdf_timer_t *timer = (void *)addr; + + timer->callback(timer->context); +} static inline QDF_STATUS __qdf_timer_init(struct __qdf_timer_t *timer, qdf_timer_func_t func, void *arg, @@ -89,20 +94,23 @@ static inline QDF_STATUS __qdf_timer_init(struct __qdf_timer_t *timer, { struct timer_list *os_timer = &timer->os_timer; bool is_on_stack = object_is_on_stack(os_timer); - __legacy_timer_callback_t callback = (__legacy_timer_callback_t)func; - unsigned long ctx = (unsigned long)arg; + unsigned long addr = (unsigned long)timer; + + timer->callback = func; + timer->context = arg; if (type == QDF_TIMER_TYPE_SW) { if (is_on_stack) - setup_deferrable_timer_on_stack(os_timer, callback, - ctx); + setup_deferrable_timer_on_stack(os_timer, + __os_timer_shim, + addr); else - setup_deferrable_timer(os_timer, callback, ctx); + setup_deferrable_timer(os_timer, __os_timer_shim, addr); } else { if (is_on_stack) - setup_timer_on_stack(os_timer, callback, ctx); + setup_timer_on_stack(os_timer, __os_timer_shim, addr); else - setup_timer(os_timer, callback, ctx); + setup_timer(os_timer, __os_timer_shim, addr); } return QDF_STATUS_SUCCESS;