When a struct containing a flexible array is included in another struct, and there is a member after the struct-with-flex-array, there is a possibility of memory overlap. These cases must be audited [1]. See:
struct inner { ... int flex[]; };
struct outer { ... struct inner header; int overlap; ... };
This is the scenario for the "struct audio_apbridgea_hdr" structure that is included in the following "struct audio_apbridgea_*_request" structures:
struct audio_apbridgea_set_config_request struct audio_apbridgea_register_cport_request struct audio_apbridgea_unregister_cport_request struct audio_apbridgea_set_tx_data_size_request struct audio_apbridgea_prepare_tx_request struct audio_apbridgea_start_tx_request struct audio_apbridgea_stop_tx_request struct audio_apbridgea_shutdown_tx_request struct audio_apbridgea_set_rx_data_size_request struct audio_apbridgea_prepare_rx_request struct audio_apbridgea_start_rx_request struct audio_apbridgea_stop_rx_request struct audio_apbridgea_shutdown_rx_request
The pattern is like the one shown below:
struct audio_apbridgea_hdr { ... __u8 data[]; } __packed;
struct audio_apbridgea_*_request { struct audio_apbridgea_hdr hdr; ... } __packed;
In this case, the trailing flexible array can be removed because it is never used.
Link: https://github.com/KSPP/linux/issues/202 [1] Signed-off-by: Erick Archer erick.archer@gmx.com --- Hi everyone,
I'm not sure this patch is correct. My concerns are:
The "struct audio_apbridgea_hdr" structure is used as a first member in all the "struct audio_apbridgea_*_request" structures. And these last structures are used in the "gb_audio_apbridgea_*(...)" functions. These functions fill the "request" structure and always use:
gb_hd_output(connection->hd, &req, sizeof(req), GB_APB_REQUEST_AUDIO_CONTROL, true);
Then, the "gb_hd_output(struct gb_host_device *hd, ...)" function calls:
hd->driver->output(hd, req, size, cmd, async);
The first parameter to this function is of type:
struct gb_host_device { ... const struct gb_hd_driver *driver; ... };
And the "gb_hd_driver" structure is defined as:
struct gb_hd_driver { ... int (*output)(struct gb_host_device *hd, void *req, u16 size, u8 cmd, bool async); };
Therefore, my question is: Where is the "output" function pointer set? I think I'm missing something.
I have search for another greybus drivers and I have found that, for example, the "es2_ap_driver" (drivers/greybus/es2.c) sets the "output" member in:
static struct gb_hd_driver es2_driver = { ... .output = output, };
I think that the flexible array that I have removed should be used in the function assigned to the "output" function pointer. But I am not able to find this function.
A bit of light on this would be greatly appreciated.
Thanks, Erick --- drivers/staging/greybus/audio_apbridgea.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/staging/greybus/audio_apbridgea.h b/drivers/staging/greybus/audio_apbridgea.h index efec0f815efd..ab707d310129 100644 --- a/drivers/staging/greybus/audio_apbridgea.h +++ b/drivers/staging/greybus/audio_apbridgea.h @@ -65,7 +65,6 @@ struct audio_apbridgea_hdr { __u8 type; __le16 i2s_port; - __u8 data[]; } __packed;
struct audio_apbridgea_set_config_request { -- 2.25.1