Commit 1b34cbbf4f01 ("crypto: af_alg - Disallow concurrent writes in af_alg_sendmsg") changed some fields from bool to 1-bit bitfields. However, some assignments to these fields, specifically 'more' and 'merge', assign values greater than 1. These relied on C's implicit conversion to bool, such that zero becomes false and nonzero becomes true. With a 1-bit bitfield instead, mod 2 of the value is taken instead, resulting in 0 being assigned in some cases when 1 was intended. Fix this by restoring the bool type.
Fixes: 1b34cbbf4f01 ("crypto: af_alg - Disallow concurrent writes in af_alg_sendmsg") Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers ebiggers@kernel.org --- include/crypto/if_alg.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h index 0c70f3a55575..02fb7c1d9ef7 100644 --- a/include/crypto/if_alg.h +++ b/include/crypto/if_alg.h @@ -150,15 +150,15 @@ struct af_alg_ctx { struct crypto_wait wait;
size_t used; atomic_t rcvused;
- u32 more:1, - merge:1, - enc:1, - write:1, - init:1; + bool more; + bool merge; + bool enc; + bool write; + bool init;
unsigned int len;
unsigned int inflight; };
base-commit: cec1e6e5d1ab33403b809f79cd20d6aff124ccfe
On Wed, 24 Sept 2025 at 12:27, Eric Biggers ebiggers@kernel.org wrote:
u32 more:1,
merge:1,
enc:1,
write:1,
init:1;
bool more;
bool merge;
bool enc;
bool write;
bool init;
This actually packs horribly, since a 'bool' will take up a byte for each, so now those five bits take up 8 bytes of storage (because the five bytes will then cause the next field to have to be aligned too).
You could just keep the bitfield format, but change the 'u32' to 'bool' and get the best of both worlds, ie just do something like
- u32 more:1, + bool more:1,
and now you get the bit packing _and_ the automatic bool behavior.
Linus
On Wed, Sep 24, 2025 at 12:40:29PM -0700, Linus Torvalds wrote:
On Wed, 24 Sept 2025 at 12:27, Eric Biggers ebiggers@kernel.org wrote:
u32 more:1,
merge:1,
enc:1,
write:1,
init:1;
bool more;
bool merge;
bool enc;
bool write;
bool init;
This actually packs horribly, since a 'bool' will take up a byte for each, so now those five bits take up 8 bytes of storage (because the five bytes will then cause the next field to have to be aligned too).
You could just keep the bitfield format, but change the 'u32' to 'bool' and get the best of both worlds, ie just do something like
u32 more:1,
bool more:1,
and now you get the bit packing _and_ the automatic bool behavior.
Sure, I'll send out v2 with your suggestion.
I do think the idea of trying to re-pack the structure as part of a bug fix is a bit misguided, though. It's what caused this additional bug in the first place, and it's not like it actually matters here. (AF_ALG is rarely used, and when it is, the sockets tend not to be kept open for very long. And the entire concept of AF_ALG is a mistake anyway.)
- Eric
On Wed, 24 Sept 2025 at 13:13, Eric Biggers ebiggers@kernel.org wrote:
I do think the idea of trying to re-pack the structure as part of a bug fix is a bit misguided, though.
Well, now it's done, so let's not change it even *more*, when a one-liner should fix it.
I do agree that clearly the original fix was clearly buggy, but unless it's reverted entirely I'd rather go for "minimal one-liner fix on top of buggy fix", particularly since the end result is then better...
Linus
linux-stable-mirror@lists.linaro.org