netdev - Re: [v5 PATCH 11/14] xfrm: ipcomp: Use crypto

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aAt8AIiFWZZwgCyj@krikkit>
Date: Fri, 25 Apr 2025 14:11:44 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
	Richard Weinberger <richard@....at>,
	Zhihao Cheng <chengzhihao1@...wei.com>,
	linux-mtd@...ts.infradead.org,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Pavel Machek <pavel@....cz>, linux-pm@...r.kernel.org,
	Steffen Klassert <steffen.klassert@...unet.com>,
	netdev@...r.kernel.org
Subject: Re: [v5 PATCH 11/14] xfrm: ipcomp: Use crypto_acomp interface

Hi Herbert,

2025-03-15, 18:30:43 +0800, Herbert Xu wrote:
> +static int ipcomp_post_acomp(struct sk_buff *skb, int err, int hlen)
> +{
> +	struct acomp_req *req = ipcomp_cb(skb)->req;
> +	struct ipcomp_req_extra *extra;
> +	const int plen = skb->data_len;
> +	struct scatterlist *dsg;
> +	int len, dlen;
>  
> -	len = dlen - plen;
> -	if (len > skb_tailroom(skb))
> -		len = skb_tailroom(skb);
> +	if (unlikely(err))
> +		goto out_free_req;
>  
> -	__skb_put(skb, len);
> +	extra = acomp_request_extra(req);
> +	dsg = extra->sg;
> +	dlen = req->dlen;
>  
> -	len += plen;
> -	skb_copy_to_linear_data(skb, scratch, len);
> +	pskb_trim_unique(skb, 0);
> +	__skb_put(skb, hlen);
>  
> -	while ((scratch += len, dlen -= len) > 0) {
> +	/* Only update truesize on input. */
> +	if (!hlen)
> +		skb->truesize += dlen - plen;

Are you sure we need to subtract plen here? When I run fragmented
traffic with ipcomp, I'm hitting the WARN from skb_try_coalesce during
reassembly, ie truesize is too small:

    delta = from->truesize - SKB_TRUESIZE(skb_end_offset(from));
    WARN_ON_ONCE(delta < len);

The splat goes away with

 	/* Only update truesize on input. */
 	if (!hlen)
-		skb->truesize += dlen - plen;
+		skb->truesize += dlen;
 	skb->data_len = dlen;
 	skb->len += dlen;

pskb_trim_unique ends up calling skb_condense, which seems to adjust
the truesize to account for all frags being dropped.

Does that look like the right fix to you?

-- 
Sabrina