| Message ID | 20231030155809.6b47288c@canb.auug.org.au (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | linux-next: build failure after merge of the crypto tree | expand |
Hi all, On Mon, 30 Oct 2023 15:58:09 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > After merging the crypto tree, today's linux-next build (powerpc > allyesconfig) failed like this: > > net/ipv4/tcp_ao.c: In function 'tcp_ao_key_alloc': > net/ipv4/tcp_ao.c:1536:13: error: implicit declaration of function 'crypto_ahash_alignmask'; did you mean 'crypto_ahash_alg_name'? [-Werror=implicit-function-declaration] > 1536 | if (crypto_ahash_alignmask(tfm) > TCP_AO_KEY_ALIGN) { > | ^~~~~~~~~~~~~~~~~~~~~~ > | crypto_ahash_alg_name > > Caused by commit > > 0f8660c82b79 ("crypto: ahash - remove crypto_ahash_alignmask") > > interacting with commit > > 4954f17ddefc ("net/tcp: Introduce TCP_AO setsockopt()s") > > from the net-next tree. > > I have applied the following merge resolution patch for today: From: Stephen Rothwell <sfr@canb.auug.org.au> Date: Mon, 30 Oct 2023 15:54:37 +1100 Subject: [PATCH] fix up for "crypto: ahash - remove crypto_ahash_alignmask" interacting with "net/tcp: Introduce TCP_AO setsockopt()s" Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- net/ipv4/tcp_ao.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c index 6a845e906a1d..ef5472ed6158 100644 --- a/net/ipv4/tcp_ao.c +++ b/net/ipv4/tcp_ao.c @@ -1533,10 +1533,6 @@ static struct tcp_ao_key *tcp_ao_key_alloc(struct sock *sk, goto err_free_pool; tfm = crypto_ahash_reqtfm(hp.req); - if (crypto_ahash_alignmask(tfm) > TCP_AO_KEY_ALIGN) { - err = -EOPNOTSUPP; - goto err_pool_end; - } digest_size = crypto_ahash_digestsize(tfm); tcp_sigpool_end(&hp); @@ -1551,8 +1547,6 @@ static struct tcp_ao_key *tcp_ao_key_alloc(struct sock *sk, key->digest_size = digest_size; return key; -err_pool_end: - tcp_sigpool_end(&hp); err_free_pool: tcp_sigpool_release(pool_id); return ERR_PTR(err);
On Mon, Oct 30, 2023 at 04:09:53PM +1100, Stephen Rothwell wrote: > > From: Stephen Rothwell <sfr@canb.auug.org.au> > Date: Mon, 30 Oct 2023 15:54:37 +1100 > Subject: [PATCH] fix up for "crypto: ahash - remove crypto_ahash_alignmask" > > interacting with "net/tcp: Introduce TCP_AO setsockopt()s" > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > --- > net/ipv4/tcp_ao.c | 6 ------ > 1 file changed, 6 deletions(-) Acked-by: Herbert Xu <herbert@gondor.apana.org.au> If we simply apply this patch to the netdev tree then everything should work at the next merge window. But perhaps you could change the patch description to say something like remove the obsolete crypto_hash_alignmask. It's not important though. Thanks,
On 10/30/23 05:23, Herbert Xu wrote: > On Mon, Oct 30, 2023 at 04:09:53PM +1100, Stephen Rothwell wrote: >> >> From: Stephen Rothwell <sfr@canb.auug.org.au> >> Date: Mon, 30 Oct 2023 15:54:37 +1100 >> Subject: [PATCH] fix up for "crypto: ahash - remove crypto_ahash_alignmask" >> >> interacting with "net/tcp: Introduce TCP_AO setsockopt()s" >> >> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> >> --- >> net/ipv4/tcp_ao.c | 6 ------ >> 1 file changed, 6 deletions(-) > > Acked-by: Herbert Xu <herbert@gondor.apana.org.au> Reviewed-by: Dmitry Safonov <dima@arista.com> > If we simply apply this patch to the netdev tree then everything > should work at the next merge window. But perhaps you could change > the patch description to say something like remove the obsolete > crypto_hash_alignmask. It's not important though. Thank you, Dmitry
On Mon, 30 Oct 2023 13:23:53 +0800 Herbert Xu wrote: > If we simply apply this patch to the netdev tree then everything > should work at the next merge window. But perhaps you could change > the patch description to say something like remove the obsolete > crypto_hash_alignmask. It's not important though. I'm happy to massage the commit message and apply the fix to net. But is it actually 100% correct to do that? IOW is calling crypto_ahash_alignmask() already not necessary in net-next or does it only become unnecessary after some prep work in crypto-next? We can tell Linus to squash this fix into the merge of either crypto-next or net-next, I'm pretty sure he'd be okay with that..
On Mon, Oct 30, 2023 at 03:02:43PM -0700, Jakub Kicinski wrote: > On Mon, 30 Oct 2023 13:23:53 +0800 Herbert Xu wrote: > > If we simply apply this patch to the netdev tree then everything > > should work at the next merge window. But perhaps you could change > > the patch description to say something like remove the obsolete > > crypto_hash_alignmask. It's not important though. > > I'm happy to massage the commit message and apply the fix to net. > But is it actually 100% correct to do that? IOW is calling > crypto_ahash_alignmask() already not necessary in net-next or does > it only become unnecessary after some prep work in crypto-next? > > We can tell Linus to squash this fix into the merge of either > crypto-next or net-next, I'm pretty sure he'd be okay with that.. It's safe to fold the patch into net-next. It actually looks like a bug to be using the alignmask in the way that net/ipv4/tcp_ao.c is using it. You don't want to be erroring out just because the algorithm declared an alignmask. - Eric
On Mon, 30 Oct 2023 21:51:57 -0700 Eric Biggers wrote: > On Mon, Oct 30, 2023 at 03:02:43PM -0700, Jakub Kicinski wrote: > > On Mon, 30 Oct 2023 13:23:53 +0800 Herbert Xu wrote: > > > If we simply apply this patch to the netdev tree then everything > > > should work at the next merge window. But perhaps you could change > > > the patch description to say something like remove the obsolete > > > crypto_hash_alignmask. It's not important though. > > > > I'm happy to massage the commit message and apply the fix to net. > > But is it actually 100% correct to do that? IOW is calling > > crypto_ahash_alignmask() already not necessary in net-next or does > > it only become unnecessary after some prep work in crypto-next? > > > > We can tell Linus to squash this fix into the merge of either > > crypto-next or net-next, I'm pretty sure he'd be okay with that.. > > It's safe to fold the patch into net-next. It actually looks like a bug to be > using the alignmask in the way that net/ipv4/tcp_ao.c is using it. You don't > want to be erroring out just because the algorithm declared an alignmask. Thanks Eric! Applied as commit f2fbb9081123 ("net: tcp: remove call to obsolete crypto_ahash_alignmask()") to net-next. I'll respin our PR after some sanity checking.
diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c index 6a845e906a1d..d18562bb3184 100644 --- a/net/ipv4/tcp_ao.c +++ b/net/ipv4/tcp_ao.c @@ -1533,10 +1533,6 @@ static struct tcp_ao_key *tcp_ao_key_alloc(struct sock *sk, goto err_free_pool; tfm = crypto_ahash_reqtfm(hp.req); - if (crypto_ahash_alignmask(tfm) > TCP_AO_KEY_ALIGN) { - err = -EOPNOTSUPP; - goto err_pool_end; - } digest_size = crypto_ahash_digestsize(tfm); tcp_sigpool_end(&hp);
Hi all, After merging the crypto tree, today's linux-next build (powerpc allyesconfig) failed like this: net/ipv4/tcp_ao.c: In function 'tcp_ao_key_alloc': net/ipv4/tcp_ao.c:1536:13: error: implicit declaration of function 'crypto_ahash_alignmask'; did you mean 'crypto_ahash_alg_name'? [-Werror=implicit-function-declaration] 1536 | if (crypto_ahash_alignmask(tfm) > TCP_AO_KEY_ALIGN) { | ^~~~~~~~~~~~~~~~~~~~~~ | crypto_ahash_alg_name Caused by commit 0f8660c82b79 ("crypto: ahash - remove crypto_ahash_alignmask") interacting with commit 4954f17ddefc ("net/tcp: Introduce TCP_AO setsockopt()s") from the net-next tree. I have applied the following merge resolution patch for today: From: Stephen Rothwell <sfr@canb.auug.org.au> Date: Mon, 30 Oct 2023 15:54:37 +1100 Subject: [PATCH] fix up for "crypto: ahash - remove crypto_ahash_alignmask" interacting with "net/tcp: Introduce TCP_AO setsockopt()s" Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- net/ipv4/tcp_ao.c | 4 ---- 1 file changed, 4 deletions(-)