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]

Date: Tue, 12 Sep 2023 17:37:23 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: netdev@...r.kernel.org, davejwatson@...com, kuba@...nel.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

2023-09-12, 12:43:32 +0800, Herbert Xu wrote:
> On Fri, Sep 08, 2023 at 05:55:59PM +0200, Sabrina Dubroca wrote:
> >
> > Uh, ok, I didn't know that, thanks for explaining. When I was fixing
> > this code I couldn't find a mention of what the expectations for
> > MAY_BACKLOG are. Could you add a comment describing this in the
> > headers (either for #define CRYPTO_TFM_REQ_MAY_BACKLOG or
> > aead_request_set_callback, wherever is more appropriate). MAY_BACKLOG
> > is used by both tls and tipc (talking only about networking) and
> > neither seem to respect this need to back off.
> 
> Patches are welcome :)

Ok. I thought it'd be better if you wrote that patch since if I write
it, it'll be a c/p (or rephrase) of what you wrote. But fine, I'll go
ahead and do that :)

> A bit of history: at the beginning we always dropped requests
> that we couldn't queue because the only user was IPsec so this
> is the expected behaviour.
> 
> When storage crypto support was added there was a need for reliable
> handling of resource constraints so that's why MAY_BACKLOG was added.
> However, the expectation is obviously that you must stop sending new
> requests once you run into the resource constraint.
> 
> > 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.
> 
> I think that would be the prudent action.

We'd have to do pretty much what Jakub suggested [1] (handle ENOSPC by
waiting for all current requests) and then resubmit the failed
request. So I think keeping the MAY_BACKLOG flag and handling EBUSY
this way is simpler. With this, we send one request to the backlog,
then we wait until the queue drains.

[1] https://lore.kernel.org/netdev/20230908142602.2ced0631@kernel.org/

-- 
Sabrina