On Mon, Apr 09, 2018 at 10:58:08AM +0200, Steffen Klassert wrote:
> On Sun, Apr 08, 2018 at 03:55:28PM -0700, Eric Biggers wrote:
> > On Fri, Mar 23, 2018 at 08:21:52AM +0800, Herbert Xu wrote:
> > > On Sat, Mar 10, 2018 at 03:22:31PM -0800, Eric Biggers wrote:
> > > > From: Eric Biggers <ebigg...@google.com>
> > > >
> > > > If the pcrypt template is used multiple times in an algorithm, then a
> > > > deadlock occurs because all pcrypt instances share the same
> > > > padata_instance, which completes requests in the order submitted. That
> > > > is, the inner pcrypt request waits for the outer pcrypt request while
> > > > the outer request is already waiting for the inner.
> > > >
> > > > Fix this by making pcrypt forbid instantiation if pcrypt appears in the
> > > > underlying ->cra_driver_name. This is somewhat of a hack, but it's a
> > > > simple fix that should be sufficient to prevent the deadlock.
> > >
> > > I'm a bit uneasy with this fix. What if pcrypt is used in the
> > > underlying algorithm as a fallback? Wouldn't it still dead-lock
> > > without triggering this check?
> > >
> > Yep, I think you're right.
> > Steffen, how feasible do you think would it be to make each pcrypt instance
> > use
> > its own padata instance, so different pcrypt instances aren't ordered with
> > respect to each other?
> It should be possible at least, but requires some work.
> I already had patches to get multiple independent pcrypt
> insatance to better support pcrypt for different workloads
> and NUMA mchines. I never got finalized with is because
> of the lack of NUMA hardware to test but the code is
> still availabe, maybe it can help:
> > Or do you think there is a better solution?
> Not really.
> Btw. is there a valid usecase where a certain template
> is used twice in one algorithm instance?
None that I can think of right now. The problem is that it's hard to prevent
template recursion when users can specify arbitrary algorithms using AF_ALG, and
some algorithms even use fallbacks which may use templates not explicitly listed
in the driver name.
Remember, a kernel deadlock is a bug even if it's "not a valid use case"...
In this case it can even be triggered by an unprivileged user via AF_ALG.