On Mon, Feb 13, 2023 at 09:52:23PM +0100, Amadeusz Sławiński wrote: > The kernel is globally removing the ambiguous 0-length and 1-element > arrays in favor of flexible arrays, so that we can gain both compile-time > and run-time array bounds checking[1]. In this instance, struct > skl_cpr_cfg contains struct skl_cpr_gtw_cfg, which defined "config_data" > as a 1-element array. > > However, case present in sound/soc/intel/skylake/skl-topology.h is not a > simple one as the structure takes part in IPC communication. Apparently > original definition missed one field, which while not used by AudioDSP > firmware when there is no additional data, is still expected to be part > of an IPC message. Currently this works because of how 'config_data' is > declared: 'config_data[1]'. Now when one replaces it with a flexible > array there would be one field missing. Update struct declaration to fix > this. > > Reported-by: Sasa Ostrouska <casaxa@xxxxxxxxx> > Link: https://lore.kernel.org/all/CALFERdwvq5day_sbDfiUsMSZCQu9HG8-SBpOZDNPeMdZGog6XA@xxxxxxxxxxxxxx/ > Cc: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > Cc: Liam Girdwood <liam.r.girdwood@xxxxxxxxxxxxxxx> > Cc: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx> > Cc: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx> > Cc: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx> > Cc: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> > Cc: Mark Brown <broonie@xxxxxxxxxx> > Cc: Jaroslav Kysela <perex@xxxxxxxx> > Cc: Takashi Iwai <tiwai@xxxxxxxx> > Cc: "Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx> > Cc: alsa-devel@xxxxxxxxxxxxxxxx > CC: Kees Cook <keescook@xxxxxxxxxxxx> > Reviewed-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx> > Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx> > --- > sound/soc/intel/skylake/skl-messages.c | 2 +- > sound/soc/intel/skylake/skl-topology.h | 5 ++++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c > index 5ab0917a2b3d..d31509298a0a 100644 > --- a/sound/soc/intel/skylake/skl-messages.c > +++ b/sound/soc/intel/skylake/skl-messages.c > @@ -549,7 +549,7 @@ static void skl_copy_copier_caps(struct skl_module_cfg *mconfig, > if (mconfig->formats_config[SKL_PARAM_INIT].caps_size == 0) > return; > > - memcpy(cpr_mconfig->gtw_cfg.config_data, > + memcpy(&cpr_mconfig->gtw_cfg.config_data, Unfortunately, this is going to run afoul of a compiler bug. :( GCC is still working on getting it fixed (and Clang will follow). But for now, this will just result in a run-time warning instead, since memcpy won't be able to "see through" the fact that "config_data" ends with a flexible array, meaning it will think it has a 4 byte size: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832 > mconfig->formats_config[SKL_PARAM_INIT].caps, > mconfig->formats_config[SKL_PARAM_INIT].caps_size); > > diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h > index 6db0fd7bad49..30a0977af943 100644 > --- a/sound/soc/intel/skylake/skl-topology.h > +++ b/sound/soc/intel/skylake/skl-topology.h > @@ -115,7 +115,10 @@ struct skl_cpr_gtw_cfg { > u32 dma_buffer_size; > u32 config_length; > /* not mandatory; required only for DMIC/I2S */ > - u32 config_data[1]; > + struct { > + u32 gtw_attrs; > + u32 data[]; > + } config_data; > } __packed; I recommend leaving the original memcpy() as it was, and instead creating an anonymous union in place of "config_data": union { u32 gtw_attrs; DECLARE_FLEX_ARRAY(u32, data); }; > > struct skl_dma_control { > -- > 2.34.1 > -- Kees Cook
