Am Samstag, 10. Juni 2017, 05:13:16 CEST schrieb Herbert Xu:

Hi Herbert,

> On Tue, May 23, 2017 at 04:31:59PM +0200, Stephan Müller wrote:
> >  static void skcipher_sock_destruct(struct sock *sk)
> >  {
> >  
> >     struct alg_sock *ask = alg_sk(sk);
> >     struct skcipher_ctx *ctx = ask->private;
> > 
> > -   struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(&ctx->req);
> > +   struct sock *psk = ask->parent;
> > +   struct alg_sock *pask = alg_sk(psk);
> > +   struct skcipher_tfm *skc = pask->private;
> > +   struct crypto_skcipher *tfm = skc->skcipher;
> > 
> > -   if (atomic_read(&ctx->inflight))
> > -           skcipher_wait(sk);
> > +   /* Suspend caller if AIO operations are in flight. */
> > +   wait_event_interruptible(skcipher_aio_finish_wait,
> > +                            (ctx->inflight == 0));
> 
> This doesn't look right.  If a signal comes in wouldn't you end
> up freeing live memory?

Right. Shouldn't we drop the ctx->inflight completely?

The code in the current patch set contains:

when an async operation is queued:

                sock_hold(sk);
                ctx->inflight++;

upon completion of the callback:

        __sock_put(sk);
        ctx->inflight--;

Thus, the socket is grabbed already. Hence, when dropping the inflight code 
including the wait queue entirely, I would think we are still save as we hold 
the socket.

Ciao
Stephan

Reply via email to