netdev - Re: [PATCH net-next] xfrm: hold device only for the asynchronous decryption

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <178469f2-f3ae-4de7-9d5f-b315c6fbca96@nvidia.com>
Date: Wed, 18 Jun 2025 18:12:12 +0800
From: Jianbo Liu <jianbol@...dia.com>
To: Herbert Xu <herbert@...dor.apana.org.au>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>, <kuba@...nel.org>,
	<steffen.klassert@...unet.com>, Cosmin Ratiu <cratiu@...dia.com>, "Eric
 Dumazet" <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
	<horms@...nel.org>
Subject: Re: [PATCH net-next] xfrm: hold device only for the asynchronous
 decryption



On 6/18/2025 5:44 PM, Herbert Xu wrote:
> On Wed, Jun 18, 2025 at 12:25:49PM +0300, Jianbo Liu wrote:
>>
>> @@ -649,18 +650,16 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
>>   		XFRM_SKB_CB(skb)->seq.input.low = seq;
>>   		XFRM_SKB_CB(skb)->seq.input.hi = seq_hi;
>>   
>> -		dev_hold(skb->dev);
>> -
>>   		if (crypto_done)
>>   			nexthdr = x->type_offload->input_tail(x, skb);
>>   		else
>>   			nexthdr = x->type->input(x, skb);
>>   
>> -		if (nexthdr == -EINPROGRESS)
>> +		if (nexthdr == -EINPROGRESS) {
>> +			dev_hold(skb->dev);
> 
> You can't do that.  The moment x->type->input is called the ref count
> can be released on another CPU.  So incrementing the refcount after
> its return is too late.
> 

OK. How about moving dev_hold before x->type->input?

@@ -503,6 +503,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, 
__be32 spi, int encap_type)
  		/* An encap_type of -1 indicates async resumption. */
  		if (encap_type == -1) {
  			async = 1;
+			dev_put(skb->dev);
  			seq = XFRM_SKB_CB(skb)->seq.input.low;
  			goto resume;
  		}
@@ -649,18 +650,18 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, 
__be32 spi, int encap_type)
  		XFRM_SKB_CB(skb)->seq.input.low = seq;
  		XFRM_SKB_CB(skb)->seq.input.hi = seq_hi;

-		dev_hold(skb->dev);
-
-		if (crypto_done)
+		if (crypto_done) {
  			nexthdr = x->type_offload->input_tail(x, skb);
-		else
+		} else {
+			dev_hold(skb->dev);
  			nexthdr = x->type->input(x, skb);
+		}

  		if (nexthdr == -EINPROGRESS)
  			return 0;
+		else if (!crypto_done)
+			dev_put(skb->dev);
  resume:
-		dev_put(skb->dev);
-
  		spin_lock(&x->lock);
  		if (nexthdr < 0) {
  			if (nexthdr == -EBADMSG) {

Thanks!
Jianbo