linux-kernel - Re: [PATCH] ASoC: ti: omap-mcbsp: duplicate sysfs failure after PROBE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Jun 2022 17:08:24 +0100
From:   Mark Brown <broonie@...nel.org>
To:     David Owens <dowens@...cisionplanting.com>
Cc:     Peter Ujfalusi <peter.ujfalusi@...il.com>,
        Jarkko Nikula <jarkko.nikula@...mer.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ASoC: ti: omap-mcbsp: duplicate sysfs failure after
 PROBE_DEFER

On Tue, Jun 14, 2022 at 10:59:31AM -0500, David Owens wrote:

> The call to sdma_pcm_platform_register() can return PROBE_DEFER, leading
> to omap_mcbsp_init() being called multiple times.  sysfs node creation
> fails in subsequent calls to omap_mcbsp_init(), which prevents
> the driver from ever successfully probing.  The resulting errors can be
> seen during boot:
> 
> [    1.749328] sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres'
> [    1.759643] CPU: 0 PID: 6 Comm: kworker/u2:0 Not tainted 5.18.0-yocto-standard #1
> [    1.767181] Hardware name: Generic OMAP36xx (Flattened Device Tree)
> [    1.773498] Workqueue: events_unbound deferred_probe_work_func
> [    1.779449]  unwind_backtrace from show_stack+0x10/0x14
> [    1.784729]  show_stack from sysfs_warn_dup+0x4c/0x60

Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative (it often is
for search engines if nothing else) then it's usually better to pull out
the relevant sections.

> +++ b/sound/soc/ti/omap-mcbsp.c
> @@ -1403,6 +1403,10 @@ static int asoc_mcbsp_probe(struct platform_device *pdev)
>         mcbsp->dev = &pdev->dev;
>         platform_set_drvdata(pdev, mcbsp);
> 
> +       ret = sdma_pcm_platform_register(&pdev->dev, "tx", "rx");
> +       if (ret)
> +               return ret;
> +
>         ret = omap_mcbsp_init(pdev);
>         if (ret)
>                 return ret;
> @@ -1412,13 +1416,9 @@ static int asoc_mcbsp_probe(struct platform_device *pdev)
>                 omap_mcbsp_dai.capture.formats = SNDRV_PCM_FMTBIT_S16_LE;
>         }
> 
> -       ret = devm_snd_soc_register_component(&pdev->dev,
> +       return devm_snd_soc_register_component(&pdev->dev,
>                                               &omap_mcbsp_component,
>                                               &omap_mcbsp_dai, 1);
> -       if (ret)
> -               return ret;
> -
> -       return sdma_pcm_platform_register(&pdev->dev, "tx", "rx");
>  }

It's not clear to me how this fixes the problem, your commit message
doesn't mention how?  I was expecting to see more error handling paths
being added to unwind the sysfs allocation, or a conversion to devm.  As
things stand it's not clear to me that the error won't persist in the
case where we defer registering the component.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)