Re: [v3 PATCH] crypto: api - fix unexpectedly getting generic implementation

Eric Biggers
2019-12-05T03:43:05Z

On Thu, Dec 05, 2019 at 09:58:11AM +0800, Herbert Xu wrote:
> On Wed, Dec 04, 2019 at 09:22:44AM -0800, Eric Biggers wrote:
> >
> > I was going to do something like this originally (but also checking that 'q' is
> > not "moribund", is a test larval, and has compatible cra_flags). But I don't
>
> You are right. I'll add these tests to the patch.
>
> > think it will work because a higher priority implementation could be registered
> > while a lower priority one is being instantiated and tested. Based on this
> > logic, when the lower priority implementation finishes being tested,
> > larval->adult wouldn't be set since a higher priority implementation is still
> > being tested. But then cryptomgr_probe() will complete() the larval anyway and
> > for the user crypto_alloc_foo() will fail with ENOENT.
>
> I think this is a different problem, one which we probably should
> address but it already exists regardless of what we do here. For
> example, assuming that tmpl(X) does not currently exist, and I
> request tmpl(X-generic) then tmpl(X-generic) along with X-generic
> will be created in the system. If someone then comes along and
> asks for tmpl(X) then we'll simply give them tmpl(X-generic) even
> if there exists an accelerated version of X.
>
> The problem you describe is simply a racy version of the above
> scenario where the requests for tmpl(X) and tmpl(X-generic) occur
> about the same time.
>

No, the problem I'm talking about is different and is new to your patch. If
tmpl(X-accelerated) is registered while someone is doing crypto_alg_mod_lookup()
that triggered instantiation of tmpl(X-generic), then crypto_alg_mod_lookup()
could fail with ENOENT, instead of returning tmpl(X-generic) as it does
currently. This is because the proposed new logic will not fulfill the request
larval if a better implementation of tmpl(X) is still being tested. But there's
no guarantee that tmpl(X) will finish being tested by the time cryptomgr_probe()
thinks it is done and complete()s the request larval with 'adult' still NULL.

(I think; I haven't actually tested this, this analysis is just based on my
reading of the code...)

- Eric