netdev - Re: [PATCH net 1/5] net: tls: handle -EBUSY on async encrypt/decrypt requests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230908142602.2ced0631@kernel.org>
Date: Fri, 8 Sep 2023 14:26:02 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: Herbert Xu <herbert@...dor.apana.org.au>, netdev@...r.kernel.org,
 vakul.garg@....com, borisp@...dia.com, john.fastabend@...il.com
Subject: Re: [PATCH net 1/5] net: tls: handle -EBUSY on async
 encrypt/decrypt requests

On Fri, 8 Sep 2023 17:55:59 +0200 Sabrina Dubroca wrote:
> Jakub, I guess we should drop the CRYPTO_TFM_REQ_MAY_BACKLOG for net,
> and maybe consider adding it back (with the back off) in
> net-next. Probably not urgent considering that nobody seems to have
> run into this bug so far.

Someone did mention it long time ago, but I don't recall the context :S
I think it was something about the device queue filling up..

> But then we have to handle ENOSPC a bit more gracefully, because right
> now it looks like
>  - on TX, we break the socket (tls_err_abort when tls_do_encryption returns
>    an error)
>  - on RX, we also break the socket, and we don't decrement
>    decrypt_pending so the recv() call gets stuck
> 
> Not sure how complex the changes would be, the sendmsg and recvmsg
> code is already a bit hard to follow.

To keep it simple we can wait for all in-flight requests to drain if we
hit EBUSY? Basically factor this bit out:

		spin_lock_bh(&ctx->decrypt_compl_lock);
		reinit_completion(&ctx->async_wait.completion);
		pending = atomic_read(&ctx->decrypt_pending);
		spin_unlock_bh(&ctx->decrypt_compl_lock);
		ret = 0;
		if (pending)
			ret = crypto_wait_req(-EINPROGRESS, &ctx->async_wait);

and call it after we get EBUSY? We'll drain the pending queue all the
way to empty, which may not be too great for throughput, but who cares
- right now we don't handle EBUSY at all, so it must be extremely rare.