Messages in this thread |  | | Date | Tue, 5 May 2026 03:55:38 +0900 | | From | Hyunwoo Kim <> | | Subject | Re: [PATCH net v2] xfrm: esp: avoid in-place decrypt on shared skb frags |
| |
On Mon, May 04, 2026 at 08:47:04PM +0200, Steffen Klassert wrote: > On Tue, May 05, 2026 at 03:01:15AM +0900, Hyunwoo Kim wrote: > > On Mon, May 04, 2026 at 11:27:12PM +0800, HexRabbit wrote: > > > From: Kuan-Ting Chen <h3xrabbit@gmail.com> > > > > > > MSG_SPLICE_PAGES can attach pages from a pipe directly to an skb. TCP > > > marks such skbs with SKBFL_SHARED_FRAG after skb_splice_from_iter(), > > > so later paths that may modify packet data can first make a private > > > copy. The IPv4/IPv6 datagram append paths did not set this flag when > > > splicing pages into UDP skbs. > > > > > > That leaves an ESP-in-UDP packet made from shared pipe pages looking > > > like an ordinary uncloned nonlinear skb. ESP input then takes the no-COW > > > fast path for uncloned skbs without a frag_list and decrypts in place > > > over data that is not owned privately by the skb. > > > > > > Mark IPv4/IPv6 datagram splice frags with SKBFL_SHARED_FRAG, matching > > > TCP. Also make ESP input fall back to skb_cow_data() when the flag is > > > present, so ESP does not decrypt externally backed frags in place. > > > Private nonlinear skb frags still use the existing fast path. > > > > > > This intentionally does not change ESP output. In esp_output_head(), > > > the path that appends the ESP trailer to existing skb tailroom without > > > calling skb_cow_data() is not reachable for nonlinear skbs: > > > skb_tailroom() returns zero when skb->data_len is nonzero, while ESP > > > tailen is positive. Thus ESP output will either use the separate > > > destination-frag path or fall back to skb_cow_data(). > > > > > > Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible") > > > Fixes: 03e2a30f6a27 ("esp6: Avoid skb_cow_data whenever possible") > > > Fixes: 7da0dde68486 ("ip, udp: Support MSG_SPLICE_PAGES") > > > Fixes: 6d8192bd69bb ("ip6, udp6: Support MSG_SPLICE_PAGES") > > > Reported-by: Hyunwoo Kim <imv4bel@gmail.com> > > > Reported-by: Kuan-Ting Chen <h3xrabbit@gmail.com> > > > > I dynamically tested this patch and confirm it resolves the > > issue. Clean work. > > Feel free to add a Tested-by: tag. > > > One correction request before merge -- please drop the > > second Reported-by tag (your own) from the trailer. > > > > The report and patch for this issue were already posted on > > the public netdev ML 6 days ago, i.e., the bug was already > > publicly reported: > > > > https://lore.kernel.org/all/afLDKSvAvMwGh7Fy@v4bel/ > > > > Credit for patch authorship is adequately covered by > > Signed-off-by alone. Setting aside that your work proceeded > > independently rather than as a review of my earlier > > submission, the trailer should conform to convention to > > avoid future misunderstanding. > > The issue was reported independently, so both Reported-by tags > are valid. But indeed the Signed-off-by tag should cover the > second one. I've applied it to the testing branch of the ipsec > tree to make it available to our test systems. I can still fix > the tags on request, no need for a v3. > > > No objections to the patch itself. > > Thanks a lot for your effort!
Hi Steffen,
Thanks for the Tested-by tag offer -- please add:
Tested-by: Hyunwoo Kim <imv4bel@gmail.com>
And please drop the second Reported-by: line as well. Appreciate the careful handling.
Best regards, Hyunwoo Kim
|  |