The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 2/5] net/rxrpc: Use local FCrypt-PCBC implementation
       [not found] ` <20260428024400.123337-3-ebiggers@kernel.org>
@ 2026-05-08 18:02   ` David Howells
  2026-05-08 18:23     ` Eric Biggers
  0 siblings, 1 reply; 3+ messages in thread
From: David Howells @ 2026-05-08 18:02 UTC (permalink / raw)
  To: Eric Biggers
  Cc: dhowells, netdev, linux-afs, Marc Dionne, linux-crypto,
	linux-kernel, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

Eric Biggers <ebiggers@kernel.org> wrote:

> +	if (skb_linearize(skb) < 0)
> +		return -ENOMEM;

It seems skb_linearize() doesn't like being used in this fashion:

	kernel BUG at net/core/skbuff.c:2295!
	...
	RIP: 0010:pskb_expand_head+0x41/0x220
	 __pskb_pull_tail+0x5e/0x2f0
	 rxkad_verify_packet_2+0xa8/0x190
	 rxkad_verify_packet+0x12c/0x150
	 rxrpc_recvmsg_data+0x1b0/0x470
	 rxrpc_kernel_recv_data+0xa6/0x210
	 afs_extract_data+0x5e/0x180
	 yfs_deliver_fs_fetch_data64+0x10b/0x200
	 afs_deliver_to_call+0xea/0x440
	 afs_read_receive+0x8d/0x150
	 afs_fetch_data_async_rx+0x12/0x20
	 process_one_work+0x18e/0x2b0

which corresponds to this:

	BUG_ON(skb_shared(skb));

Presumably this is done because fcrypt_pcbc_decrypt() doesn't handle being
called on a split buffer.  I think this may require skb_copy() to be used
instead, but that would need to be handled in rxrpc_input_call_event().

I think rxkad_decrypt_response() should be okay because the encrypted data is
extracted into a buffer first before being decrypted.

David


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next 2/5] net/rxrpc: Use local FCrypt-PCBC implementation
  2026-05-08 18:02   ` [PATCH net-next 2/5] net/rxrpc: Use local FCrypt-PCBC implementation David Howells
@ 2026-05-08 18:23     ` Eric Biggers
  2026-05-08 18:56       ` David Howells
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Biggers @ 2026-05-08 18:23 UTC (permalink / raw)
  To: David Howells
  Cc: netdev, linux-afs, Marc Dionne, linux-crypto, linux-kernel,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman

On Fri, May 08, 2026 at 07:02:05PM +0100, David Howells wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > +	if (skb_linearize(skb) < 0)
> > +		return -ENOMEM;
> 
> It seems skb_linearize() doesn't like being used in this fashion:
> 
> 	kernel BUG at net/core/skbuff.c:2295!
> 	...
> 	RIP: 0010:pskb_expand_head+0x41/0x220
> 	 __pskb_pull_tail+0x5e/0x2f0
> 	 rxkad_verify_packet_2+0xa8/0x190
> 	 rxkad_verify_packet+0x12c/0x150
> 	 rxrpc_recvmsg_data+0x1b0/0x470
> 	 rxrpc_kernel_recv_data+0xa6/0x210
> 	 afs_extract_data+0x5e/0x180
> 	 yfs_deliver_fs_fetch_data64+0x10b/0x200
> 	 afs_deliver_to_call+0xea/0x440
> 	 afs_read_receive+0x8d/0x150
> 	 afs_fetch_data_async_rx+0x12/0x20
> 	 process_one_work+0x18e/0x2b0
> 
> which corresponds to this:
> 
> 	BUG_ON(skb_shared(skb));
> 
> Presumably this is done because fcrypt_pcbc_decrypt() doesn't handle being
> called on a split buffer.  I think this may require skb_copy() to be used
> instead, but that would need to be handled in rxrpc_input_call_event().
> 
> I think rxkad_decrypt_response() should be okay because the encrypted data is
> extracted into a buffer first before being decrypted.

Yes, Marc pointed this out already.  Thanks for the review and testing.
I just haven't had a chance to decide what to do with this patch yet.
It could be an unconditional skb_copy(), it could be decrypting the
fragmented skb in-place, or it could be fixing up the RxRPC code to no
longer take multiple references to the skb (so skb_shared() would no
longer be true).  Let me know if you have a preference.

Also I'm waiting to see if the following patch gets merged:
https://lore.kernel.org/netdev/20260502211340.446927-1-n7l8m4@u.northwestern.edu/
That does the skb_copy() anyway, which would solve this problem.

- Eric

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next 2/5] net/rxrpc: Use local FCrypt-PCBC implementation
  2026-05-08 18:23     ` Eric Biggers
@ 2026-05-08 18:56       ` David Howells
  0 siblings, 0 replies; 3+ messages in thread
From: David Howells @ 2026-05-08 18:56 UTC (permalink / raw)
  To: Eric Biggers
  Cc: dhowells, netdev, linux-afs, Marc Dionne, linux-crypto,
	linux-kernel, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

Eric Biggers <ebiggers@kernel.org> wrote:

> Also I'm waiting to see if the following patch gets merged:
> https://lore.kernel.org/netdev/20260502211340.446927-1-n7l8m4@u.northwestern.edu/

This is the favoured solution:

	https://lore.kernel.org/netdev/af2kdW2F1gJ9U-Gg@v4bel/

The problem with the one you mentioned is that it does a mandatory copy, even
when it doesn't need to, for rxgk.  I can benchmark that to see what the
performance impact it has.

David


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-08 18:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260428024400.123337-1-ebiggers@kernel.org>
     [not found] ` <20260428024400.123337-3-ebiggers@kernel.org>
2026-05-08 18:02   ` [PATCH net-next 2/5] net/rxrpc: Use local FCrypt-PCBC implementation David Howells
2026-05-08 18:23     ` Eric Biggers
2026-05-08 18:56       ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox