Am Dienstag, 7. November 2017, 06:22:35 CET schrieb Herbert Xu: Hi Herbert, > On Mon, Nov 06, 2017 at 05:06:09PM +0100, Stephan Mueller wrote: > > Am Freitag, 3. November 2017, 14:20:16 CET schrieb Herbert Xu: > > > Are you sure about that? In particular is the callback function still > > > sane without the socket lock if a concurrent recvmsg/sendmsg call is > > > made? > > > > I reviewed the code again and I cannot find a reason for keeping the lock. > > All we need to ensure is that the socket exists. This is ensured with the > > refcount of the socket released by __sock_put(). > > OK, I can't see why we need a lock there either. However, the call > to __sock_put looks suspicious. Why isn't this using sock_put? I simply ported the existing code from algif_aead over -- but I think you are right that sock_put is more appropriate. > > Also the sock_hold on the caller side looks buggy. Surely it needs > to be made before we even call the encrypt/decrypt functions rather > than after it returns EINPROGRESS at which point it may well be too > late? I would concur. The sock_hold would need to be moved from the EINPROGRESS conditional to before the AIO enc/dec operation is invoked. Where I am not fully sure is whether af_alg_async_cb is called in any case. I.e. when we invoke an AIO operation with a cipher that completes synchronously (e.g. AES-NI), is this callback triggered? Ciao Stephan
