linux-kernel - Re: [PATCH] media: venus: Synchronize probe() between venus

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]

Message-ID: <25da5210-8e1f-7183-a8e7-8584f8dd2cef@linaro.org>
Date:   Mon, 25 Oct 2021 09:57:43 -0700
From:   Tadeusz Struk <tadeusz.struk@...aro.org>
To:     Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Stanimir Varbanov <stanimir.varbanov@...aro.org>,
        Andy Gross <agross@...nel.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Amit Pundir <amit.pundir@...aro.org>,
        John Stultz <john.stultz@...aro.org>,
        linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: venus: Synchronize probe() between venus_core and
 enc/dec

On 10/25/21 09:03, Bryan O'Donoghue wrote:
> I don't think there's any guarantee at all, that core probe() has completed at 
> that point.

I think there is, thanks to the new sync_mutex. The enc/dec probe will keep
returning -EPROBE_DEFER; until the core driver calls
platform_set_drvdata(pdev, core); in line 338, but before it does that it
takes the syn_lock. Then both enc/dec drivers will block on the same sync_lock
until either the core has finished initialization fully and unlocks it in line
378 just before returning 0, or it fails in between and unlocks it on the err
path. Only then the other two can proceed and check if the core probe failed,
in which case the condition core->state != CORE_INIT will be true.

> 
> of_platform_populate() doesn't guarantee ordering of the probe() completing 
> before or after the probe() of the platform drivers that are associated with the 
> devices in of_platform_populate().

agree, but I don't depend on of_platform_populate(). The ordering between the
three probe functions is enforced by the new sync mutex.

> 
> When you think it about it can't do that and you wouldn't want it to do that 
> since a device might have a legitimate reason to EPROBE_DEFER
> 
> As an example core could call of_platform_populate() and then as a ridiculous 
> example go to sleep for five seconds - in which case it is perfectly possible 

and this is exactly what happens when the core probe() loads the firmware from
disk.

> the encoder and decoder probe() functions will bug out illegitimately waiting 
> because of core->state != CORE_INIT

not really, because it will block on the mutex and only check the condition
after the sync_lock is unlocked.

-- 
Thanks,
Tadeusz

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)