Message ID | 20200601121323.GB45647@sirena.org.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [GIT,PULL] ASoC updates for v5.8 | expand |
On Mon, 01 Jun 2020 14:13:23 +0200, Mark Brown wrote: > > The following changes since commit 9cb1fd0efd195590b828b9b865421ad345a4a145: > > Linux 5.7-rc7 (2020-05-24 15:32:54 -0700) > > are available in the Git repository at: > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git tags/asoc-v5.8 > > for you to fetch changes up to 358c7c61fd04d324f83d7968daf8dd9a6ff86a9a: > > Merge remote-tracking branch 'asoc/for-5.8' into asoc-linus (2020-06-01 13:01:15 +0100) > > ---------------------------------------------------------------- > ASoC: Updates for v5.8 > > This has been another very active release with a bunch of new drivers, > lots of fixes everywhere and continued core improvements from > Morimoto-san: > > - Lots of core cleanups and refactorings from Morimoto-san, factoring > out common operations and making the card abstraction more solid. > - Continued work on cleaning up and improving the Intel drivers, along > with some new platform support for them. > - Fixes to make the Marvell SSPA driver work upstream. > - Support for AMD Renoir ACP, Dialog DA7212, Freescale EASRC and > i.MX8M, Intel Elkhard Lake, Maxim MAX98390, Nuvoton NAU8812 and > NAU8814 and Realtek RT1016. Pulled now. Thanks. Takashi
On Mon, 01 Jun 2020 20:44:55 +0200, Takashi Iwai wrote: > > On Mon, 01 Jun 2020 14:13:23 +0200, > Mark Brown wrote: > > > > The following changes since commit 9cb1fd0efd195590b828b9b865421ad345a4a145: > > > > Linux 5.7-rc7 (2020-05-24 15:32:54 -0700) > > > > are available in the Git repository at: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git tags/asoc-v5.8 > > > > for you to fetch changes up to 358c7c61fd04d324f83d7968daf8dd9a6ff86a9a: > > > > Merge remote-tracking branch 'asoc/for-5.8' into asoc-linus (2020-06-01 13:01:15 +0100) > > > > ---------------------------------------------------------------- > > ASoC: Updates for v5.8 > > > > This has been another very active release with a bunch of new drivers, > > lots of fixes everywhere and continued core improvements from > > Morimoto-san: > > > > - Lots of core cleanups and refactorings from Morimoto-san, factoring > > out common operations and making the card abstraction more solid. > > - Continued work on cleaning up and improving the Intel drivers, along > > with some new platform support for them. > > - Fixes to make the Marvell SSPA driver work upstream. > > - Support for AMD Renoir ACP, Dialog DA7212, Freescale EASRC and > > i.MX8M, Intel Elkhard Lake, Maxim MAX98390, Nuvoton NAU8812 and > > NAU8814 and Realtek RT1016. > > Pulled now. Thanks. BTW, this pull request caused a compile warning: sound/soc/codecs/max98390.c: In function ‘max98390_dsm_init’: sound/soc/codecs/max98390.c:781:3: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘size_t {aka const unsigned int}’ [-Wformat=] This should be fixed by the correct qualifier 'z' instead of 'l'. Takashi
On Mon, 01 Jun 2020 23:17:59 +0200, Takashi Iwai wrote: > > On Mon, 01 Jun 2020 20:44:55 +0200, > Takashi Iwai wrote: > > > > On Mon, 01 Jun 2020 14:13:23 +0200, > > Mark Brown wrote: > > > > > > The following changes since commit 9cb1fd0efd195590b828b9b865421ad345a4a145: > > > > > > Linux 5.7-rc7 (2020-05-24 15:32:54 -0700) > > > > > > are available in the Git repository at: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git tags/asoc-v5.8 > > > > > > for you to fetch changes up to 358c7c61fd04d324f83d7968daf8dd9a6ff86a9a: > > > > > > Merge remote-tracking branch 'asoc/for-5.8' into asoc-linus (2020-06-01 13:01:15 +0100) > > > > > > ---------------------------------------------------------------- > > > ASoC: Updates for v5.8 > > > > > > This has been another very active release with a bunch of new drivers, > > > lots of fixes everywhere and continued core improvements from > > > Morimoto-san: > > > > > > - Lots of core cleanups and refactorings from Morimoto-san, factoring > > > out common operations and making the card abstraction more solid. > > > - Continued work on cleaning up and improving the Intel drivers, along > > > with some new platform support for them. > > > - Fixes to make the Marvell SSPA driver work upstream. > > > - Support for AMD Renoir ACP, Dialog DA7212, Freescale EASRC and > > > i.MX8M, Intel Elkhard Lake, Maxim MAX98390, Nuvoton NAU8812 and > > > NAU8814 and Realtek RT1016. > > > > Pulled now. Thanks. > > BTW, this pull request caused a compile warning: > sound/soc/codecs/max98390.c: In function ‘max98390_dsm_init’: > sound/soc/codecs/max98390.c:781:3: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘size_t {aka const unsigned int}’ [-Wformat=] > > This should be fixed by the correct qualifier 'z' instead of 'l'. Since no one reacts: below is the quick fix. I'll resubmit later via git-send-email. But, looking more at the code around that line, I could spot many other bugs. You cannot trust the firmware file and you must check the size. The current code can trigger out-of-bound accesses and crash very easily when a malformed firmware file is tossed; e.g. just put an empty file (or a huge file) as dsm_param.bin. Steve, could you fix it quickly, so that we can cover it for rc1? thanks, Takashi -- 8< -- From: Takashi Iwai <tiwai@suse.de> Subject: [PATCH] ASoC: max98390: Fix incorrect printf qualifier This patch addresses a compile warning: sound/soc/codecs/max98390.c:781:3: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘size_t {aka const unsigned int}’ [-Wformat=] Fixes: a6e3f4f34cdb ("ASoC: max98390: Added Amplifier Driver") Signed-off-by: Takashi Iwai <tiwai@suse.de> --- sound/soc/codecs/max98390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/max98390.c b/sound/soc/codecs/max98390.c index b9ce44dda886..be7cd0aeb6a6 100644 --- a/sound/soc/codecs/max98390.c +++ b/sound/soc/codecs/max98390.c @@ -778,7 +778,7 @@ static int max98390_dsm_init(struct snd_soc_component *component) } dev_dbg(component->dev, - "max98390: param fw size %ld\n", + "max98390: param fw size %zd\n", fw->size); dsm_param = (char *)fw->data; dsm_param += MAX98390_DSM_PAYLOAD_OFFSET;
On Tue, Jun 02, 2020 at 06:42:25PM +0200, Takashi Iwai wrote: > But, looking more at the code around that line, I could spot many > other bugs. You cannot trust the firmware file and you must check the > size. The current code can trigger out-of-bound accesses and crash > very easily when a malformed firmware file is tossed; e.g. just put an > empty file (or a huge file) as dsm_param.bin. Yeah, it's not great - the potential impact is limited by regmap which will bounds check attempts to write beyond the last register (though now I look again we're using unsigned values so we should definitely be limiting the lower size) but it would be better to limit it to just the registers that should be being configured. Possibly even just to one valid file length if it's always all the same registers being configured. Steve?
> -----Original Message----- > From: Takashi Iwai <tiwai@suse.de> > Sent: Wednesday, June 3, 2020 1:42 AM > To: Mark Brown <broonie@kernel.org> > Cc: alsa-devel@alsa-project.org; Liam Girdwood <lgirdwood@gmail.com>; > Steve Lee <SteveS.Lee@maximintegrated.com> > Subject: Re: [GIT PULL] ASoC updates for v5.8 > > EXTERNAL EMAIL > > > > On Mon, 01 Jun 2020 23:17:59 +0200, > Takashi Iwai wrote: > > > > On Mon, 01 Jun 2020 20:44:55 +0200, > > Takashi Iwai wrote: > > > > > > On Mon, 01 Jun 2020 14:13:23 +0200, > > > Mark Brown wrote: > > > > > > > > The following changes since commit > 9cb1fd0efd195590b828b9b865421ad345a4a145: > > > > > > > > Linux 5.7-rc7 (2020-05-24 15:32:54 -0700) > > > > > > > > are available in the Git repository at: > > > > > > > > > > > > https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > > > git.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fbroonie%2Fsoun > > > > > d.git&data=02%7C01%7Csteves.lee%40maximintegrated.com%7C3dad21 > > > > > 05c5bf4f83092a08d80713ef1b%7Cfbd909dfea694788a554f24b7854ad03%7C0% > > > > > 7C0%7C637267129495484592&sdata=pVaaHm7GLdhEE6HWNPMiUbto7Bt > LJip > > > > PO8V29LLgzBg%3D&reserved=0 tags/asoc-v5.8 > > > > > > > > for you to fetch changes up to > 358c7c61fd04d324f83d7968daf8dd9a6ff86a9a: > > > > > > > > Merge remote-tracking branch 'asoc/for-5.8' into asoc-linus > > > > (2020-06-01 13:01:15 +0100) > > > > > > > > ---------------------------------------------------------------- > > > > ASoC: Updates for v5.8 > > > > > > > > This has been another very active release with a bunch of new > > > > drivers, lots of fixes everywhere and continued core improvements > > > > from > > > > Morimoto-san: > > > > > > > > - Lots of core cleanups and refactorings from Morimoto-san, factoring > > > > out common operations and making the card abstraction more solid. > > > > - Continued work on cleaning up and improving the Intel drivers, along > > > > with some new platform support for them. > > > > - Fixes to make the Marvell SSPA driver work upstream. > > > > - Support for AMD Renoir ACP, Dialog DA7212, Freescale EASRC and > > > > i.MX8M, Intel Elkhard Lake, Maxim MAX98390, Nuvoton NAU8812 and > > > > NAU8814 and Realtek RT1016. > > > > > > Pulled now. Thanks. > > > > BTW, this pull request caused a compile warning: > > sound/soc/codecs/max98390.c: In function ‘max98390_dsm_init’: > > sound/soc/codecs/max98390.c:781:3: warning: format ‘%ld’ expects > > argument of type ‘long int’, but argument 4 has type ‘size_t {aka > > const unsigned int}’ [-Wformat=] > > > > This should be fixed by the correct qualifier 'z' instead of 'l'. > > Since no one reacts: below is the quick fix. I'll resubmit later via git-send-email. > > But, looking more at the code around that line, I could spot many other bugs. > You cannot trust the firmware file and you must check the size. The current > code can trigger out-of-bound accesses and crash very easily when a malformed > firmware file is tossed; e.g. just put an empty file (or a huge file) as > dsm_param.bin. > > Steve, could you fix it quickly, so that we can cover it for rc1? > > > thanks, > > Takashi Sorry for reply late. I will fix it and update. > > -- 8< -- > From: Takashi Iwai <tiwai@suse.de> > Subject: [PATCH] ASoC: max98390: Fix incorrect printf qualifier > > This patch addresses a compile warning: > sound/soc/codecs/max98390.c:781:3: warning: format ‘%ld’ expects argument > of type ‘long int’, but argument 4 has type ‘size_t {aka const unsigned int}’ [- > Wformat=] > > Fixes: a6e3f4f34cdb ("ASoC: max98390: Added Amplifier Driver") > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > sound/soc/codecs/max98390.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/soc/codecs/max98390.c b/sound/soc/codecs/max98390.c > index b9ce44dda886..be7cd0aeb6a6 100644 > --- a/sound/soc/codecs/max98390.c > +++ b/sound/soc/codecs/max98390.c > @@ -778,7 +778,7 @@ static int max98390_dsm_init(struct > snd_soc_component *component) > } > > dev_dbg(component->dev, > - "max98390: param fw size %ld\n", > + "max98390: param fw size %zd\n", > fw->size); > dsm_param = (char *)fw->data; > dsm_param += MAX98390_DSM_PAYLOAD_OFFSET; > -- > 2.16.4
> -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Wednesday, June 3, 2020 4:04 AM > To: Takashi Iwai <tiwai@suse.de> > Cc: alsa-devel@alsa-project.org; Liam Girdwood <lgirdwood@gmail.com>; > Steve Lee <SteveS.Lee@maximintegrated.com> > Subject: Re: [GIT PULL] ASoC updates for v5.8 > > On Tue, Jun 02, 2020 at 06:42:25PM +0200, Takashi Iwai wrote: > > > But, looking more at the code around that line, I could spot many > > other bugs. You cannot trust the firmware file and you must check the > > size. The current code can trigger out-of-bound accesses and crash > > very easily when a malformed firmware file is tossed; e.g. just put an > > empty file (or a huge file) as dsm_param.bin. > > Yeah, it's not great - the potential impact is limited by regmap which will bounds > check attempts to write beyond the last register (though now I look again we're > using unsigned values so we should definitely be limiting the lower size) but it > would be better to limit it to just the registers that should be being configured. > Possibly even just to one valid file length if it's always all the same registers > being configured. > > Steve? Thanks for suggest in detail. I will update as limiting size as prevent potential risk.