> -----Original Message----- > From: Mark Brown <broo...@kernel.org> > Sent: Wednesday, June 3, 2020 8:43 PM > To: Steve Lee <steves....@maximintegrated.com> > Cc: lgirdw...@gmail.com; pe...@perex.cz; ti...@suse.com; > ckee...@opensource.cirrus.com; ge...@linux-m68k.org; > r...@opensource.wolfsonmicro.com; shumi...@realtek.com; > srinivas.kandaga...@linaro.org; k...@kernel.org; dmur...@ti.com; > jack...@realtek.com; nuno...@analog.com; linux-kernel@vger.kernel.org; > alsa-de...@alsa-project.org; ryan.lee.ma...@gmail.com; Ryan Lee > <ryans....@maximintegrated.com>; steves.lee.ma...@gmail.com > Subject: Re: [PATCH] ASoC: max98390: Fix potential crash during param fw > loading > > On Wed, Jun 03, 2020 at 11:37:44AM +0000, Steve Lee wrote: > > > > This is now reading the size out of the header of the file which is > > > good but it should also validate that the file is big enough to have > > > this much data in it, otherwise it's possible to read beyond the end > > > of the firmware file (eg, if it got truncated somehow). Previously > > > the code used the size of the file read from disk so that wasn't an issue. > > > Thanks for quick comment. Can this case cover by below line? > > + if (fw->size < MAX98390_DSM_PARAM_MIN_SIZE) { > > + dev_err(component->dev, > > + "param fw is invalid.\n"); > > + goto err_alloc; > > + } > > No, that doesn't cover all of it - the case I'm concerned about is the case > where > we've got enough data for the header but the payload is truncated. You need a > check that param_size + _PAYLOAD_OFFSET is less than fw->size as well.
Yes, I will update this and good enough.