Re: [PATCH 2/4] ALSA: hda: intel-nhlt: add intel_nhlt_ssp_mclk_mask()

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

Hi Takashi,

>> +#define SSP_BLOB_V1_0_SIZE		84
>> +#define SSP_BLOB_V1_0_MDIVC_OFFSET	19 /* offset in u32 */
>> +#define SSP_BLOB_V1_5_SIZE		96
>> +#define SSP_BLOB_V1_5_MDIVC_OFFSET	21 /* offset in u32 */
> 
> This is 84 in bytes, which is equal with SSP_BLOB_V1_0_size.
> So...
> 
>> +			for (j = 0; j < fmt->fmt_count; j++) {
>> +				u32 *blob;
>> +				int mdivc_offset;
>> +
>> +				if (cfg->config.size >= SSP_BLOB_V1_0_SIZE) {
>> +					blob = (u32 *)cfg->config.caps;
> 
> ... the size check is >= 84.  If cfg->config.size==84, it may be an
> out-of-bound read at blob[SSP_BLOB_V1_5_MDIVC_OFFSET]?
> 
> I don't think this would really matter in practice, but it's better to
> have a proper check, of course.

The check was intended to be a minimal check but you're right that it
doesn't cover the 1.5 case.

it might make more sense to first make sure we have enough space to read
the version and then check for an exact match between expected size and
actual size before reading the mdivc value.

Will fix, thanks for the feedback.
-Pierre