On Sat, Feb 17, 2024 at 03:18:59PM -0600, Alex Elder wrote:
On 2/17/24 9:47 AM, Erick Archer wrote:
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:
Yeah this was not a very good way to define these header structures, but I'm glad to hear the flexible array at the end was never used. I don't know why it was there; maybe it's an artifact from some other information that got removed.
If the code compiles with your change, it ought to be fine. (It compiles for me.)
It would be good for Vaibhav or Mark to comment though, maybe they can provide some context.
Sorry for the delay guys.
The way this was done comes from associated firmware that ran on the APBridge. This goes back a while but I think the packet headers may have been in flux at the time and this was a convenient way to change all of the packets if & when it changed. Anyway, it doesn't seem so convenient now. :)
So, yeah, getting rid of it sounds like a good thing to do to me.
I'd like to hear from these others, but otherwise this change looks good to me.
Reviewed-by: Alex Elder elder@linaro.org
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
Acked-by: Mark Greer mgreer@animalcreek.com