On Sat, Apr 28, 2018 at 8:47 AM, Stefan Wahren <[email protected]> wrote:
> The kernel would like to have all stack VLA usage removed[1]. The array
> here is fixed (declared with a const variable) but it appears like a VLA
> to the compiler. Also, currently we are putting 768 bytes on the
> stack. This function is only called on the error path so performance is
> not critical, let's just allocate the memory instead of using the
> stack. This saves stack space and removes the VLA build warning.
>
> kmalloc a buffer for dumping state instead of using the stack.
>
> [1]: https://lkml.org/lkml/2018/3/7/621
>
> CC: Kees Cook <[email protected]>
> Signed-off-by: Tobin C. Harding <[email protected]>
> Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> ---
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 25 ++++++++++++++--------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 8575bd8..d46a5b8 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -3415,13 +3415,18 @@ vchiq_release_service(VCHIQ_SERVICE_HANDLE_T handle)
> return ret;
> }
>
> +struct service_data_struct {
> + int fourcc;
> + int clientid;
> + int use_count;
> +};
> +
> void
> vchiq_dump_service_use_state(VCHIQ_STATE_T *state)
> {
> VCHIQ_ARM_STATE_T *arm_state = vchiq_platform_get_arm_state(state);
> + struct service_data_struct *service_data;
> int i, j = 0;
> - /* Only dump 64 services */
> - static const int local_max_services = 64;
> /* If there's more than 64 services, only dump ones with
> * non-zero counts */
> int only_nonzero = 0;
> @@ -3432,25 +3437,25 @@ vchiq_dump_service_use_state(VCHIQ_STATE_T *state)
> int peer_count;
> int vc_use_count;
> int active_services;
> - struct service_data_struct {
> - int fourcc;
> - int clientid;
> - int use_count;
> - } service_data[local_max_services];
>
> if (!arm_state)
> return;
>
> + service_data = kmalloc_array(MAX_SERVICES, sizeof(*service_data),
> + GFP_KERNEL);
> + if (!service_data)
> + return;
> +
> read_lock_bh(&arm_state->susp_res_lock);
> vc_suspend_state = arm_state->vc_suspend_state;
> vc_resume_state = arm_state->vc_resume_state;
> peer_count = arm_state->peer_use_count;
> vc_use_count = arm_state->videocore_use_count;
> active_services = state->unused_service;
> - if (active_services > local_max_services)
> + if (active_services > MAX_SERVICES)
> only_nonzero = 1;
>
> - for (i = 0; (i < active_services) && (j < local_max_services); i++) {
> + for (i = 0; (i < active_services) && (j < MAX_SERVICES); i++) {
> VCHIQ_SERVICE_T *service_ptr = state->services[i];
>
> if (!service_ptr)
> @@ -3494,6 +3499,8 @@ vchiq_dump_service_use_state(VCHIQ_STATE_T *state)
> vchiq_log_warning(vchiq_susp_log_level,
> "--- Overall vchiq instance use count %d", vc_use_count);
>
> + kfree(service_data);
> +
> vchiq_dump_platform_use_state(state);
> }
>
> --
> 2.7.4
>
--
Kees Cook
Pixel Security