| Message ID | 20200206200345.175344-1-caij2003@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: soc-core: fix an uninitialized use | expand |
Hi Nick,
'ret' is only defined in if branches and for loops (e.g.
for_each_component_dais). If none of these branches or loops get executed,
then eventually we end up having
int ret;
err_probe:
if (ret < 0)
soc_cleanup_component(component);
With -ftrivial-auto-var-init=pattern, this code becomes
int ret;
err_probe:
ret = 0xAAAAAAAA;
if (ret < 0)
soc_cleanup_component(component);
So soc_cleanup_component gets called unintentionally this case, which
causes the built kernel to miss some files.
On Thu, Feb 6, 2020 at 3:28 PM Nick Desaulniers <ndesaulniers@google.com>
wrote:
> > Fixed the uninitialized use of a signed integer variable ret in
> > soc_probe_component when all its definitions are not executed. This
> > caused -ftrivial-auto-var-init=pattern to initialize the variable to
> > repeated 0xAA (i.e. a negative value) and triggered the following code
> > unintentionally.
>
> > Signed-off-by: Jian Cai <caij2003@gmail.com>
>
> Hi Jian,
> I don't quite follow; it looks like `ret` is assigned to multiple times in
> `soc_probe_component`. Are one of the return values of one of the functions
> that are called then assigned to `ret` undefined? What control flow path
> leaves
> `ret` unitialized?
>
Thanks for the pointers. You are absolutely right (despite working late), this is not an issue upstream anymore. I was looking at 4.14 and 4.19 on ChromeOS. I did double check the upstream code but stopped right after seeing 'ret' was still uninitialized. Thanks again for the information. On Thu, Feb 6, 2020 at 4:04 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > On Fri, Feb 7, 2020 at 12:55 AM Jian Cai <caij2003@gmail.com> wrote: > > > > Hi Nick, > > > > 'ret' is only defined in if branches and for loops (e.g. > for_each_component_dais). If none of these branches or loops get executed, > then eventually we end up having > > https://elixir.bootlin.com/linux/latest/source/sound/soc/soc-core.c#L1276 > and > https://elixir.bootlin.com/linux/latest/source/sound/soc/soc-core.c#L1287 > both assign to `ret` before any `goto` is taken. Are you perhaps > looking at an older branch of the LTS tree, but not the master branch > of the mainline tree? (Or it's possible that it's 1am here in Zurich, > and I should go to bed). > > > > > > int ret; > > > > err_probe: > > if (ret < 0) > > soc_cleanup_component(component); > > > > With -ftrivial-auto-var-init=pattern, this code becomes > > > > int ret; > > > > err_probe: > > ret = 0xAAAAAAAA; > > if (ret < 0) > > soc_cleanup_component(component); > > > > So soc_cleanup_component gets called unintentionally this case, which > causes the built kernel to miss some files. > > > > > > > > On Thu, Feb 6, 2020 at 3:28 PM Nick Desaulniers <ndesaulniers@google.com> > wrote: > >> > >> > Fixed the uninitialized use of a signed integer variable ret in > >> > soc_probe_component when all its definitions are not executed. This > >> > caused -ftrivial-auto-var-init=pattern to initialize the variable to > >> > repeated 0xAA (i.e. a negative value) and triggered the following code > >> > unintentionally. > >> > >> > Signed-off-by: Jian Cai <caij2003@gmail.com> > >> > >> Hi Jian, > >> I don't quite follow; it looks like `ret` is assigned to multiple times > in > >> `soc_probe_component`. Are one of the return values of one of the > functions > >> that are called then assigned to `ret` undefined? What control flow > path leaves > >> `ret` unitialized? > > > > -- > Thanks, > ~Nick Desaulniers >
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 068d809c349a..bfb813ba34f3 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1180,7 +1180,7 @@ static int soc_probe_component(struct snd_soc_card *card, snd_soc_component_get_dapm(component); struct snd_soc_dai *dai; int probed = 0; - int ret; + int ret = 0; if (!strcmp(component->name, "snd-soc-dummy")) return 0;
Fixed the uninitialized use of a signed integer variable ret in soc_probe_component when all its definitions are not executed. This caused -ftrivial-auto-var-init=pattern to initialize the variable to repeated 0xAA (i.e. a negative value) and triggered the following code unintentionally. err_probe: if (ret < 0) soc_cleanup_component(component); Signed-off-by: Jian Cai <caij2003@gmail.com> --- sound/soc/soc-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)