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
