1. Patchew
  2. linux
  3. [RFC PATCH 0/2] tracking the references of net_device in aoe
  4. View patch

[RFC PATCH 2/2] aoe: using wrappers instead of dev_hold/dev_put for tracking the references of net_device in aoeif

Chun-Yi Lee posted 2 patches 1 month, 3 weeks ago
[RFC PATCH 2/2] aoe: using wrappers instead of dev_hold/dev_put for tracking the references of net_device in aoeif
Posted by Chun-Yi Lee 1 month, 3 weeks ago 
Signed-off-by: Chun-Yi Lee <jlee@suse.com>
---
 drivers/block/aoe/aoecmd.c | 24 ++++++++++++------------
 drivers/block/aoe/aoedev.c |  3 ++-
 drivers/block/aoe/aoenet.c |  2 +-
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index d1f4ddc57645..2bae364fc5ef 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -361,7 +361,7 @@ ata_rw_frameinit(struct frame *f)
 	}
 
 	ah->cmdstat = ATA_CMD_PIO_READ | writebit | extbit;
-	dev_hold(t->ifp->nd);
+	nd_dev_hold(t->ifp->nd, t->ifp);
 	skb->dev = t->ifp->nd;
 }
 
@@ -403,7 +403,7 @@ aoecmd_ata_rw(struct aoedev *d)
 		__skb_queue_tail(&queue, skb);
 		aoenet_xmit(&queue);
 	} else {
-		dev_put(f->t->ifp->nd);
+		nd_dev_put(f->t->ifp->nd, f->t->ifp);
 	}
 	return 1;
 }
@@ -421,16 +421,16 @@ aoecmd_cfg_pkts(ushort aoemajor, unsigned char aoeminor, struct sk_buff_head *qu
 
 	rcu_read_lock();
 	for_each_netdev_rcu(&init_net, ifp) {
-		dev_hold(ifp);
+		nd_dev_hold(ifp, NULL);
 		if (!is_aoe_netif(ifp)) {
-			dev_put(ifp);
+			nd_dev_put(ifp, NULL);
 			continue;
 		}
 
 		skb = new_skb(sizeof *h + sizeof *ch);
 		if (skb == NULL) {
 			printk(KERN_INFO "aoe: skb alloc failure\n");
-			dev_put(ifp);
+			nd_dev_put(ifp, NULL);
 			continue;
 		}
 		skb_put(skb, sizeof *h + sizeof *ch);
@@ -486,11 +486,11 @@ resend(struct aoedev *d, struct frame *f)
 	memcpy(h->dst, t->addr, sizeof h->dst);
 	memcpy(h->src, t->ifp->nd->dev_addr, sizeof h->src);
 
-	dev_hold(t->ifp->nd);
+	nd_dev_hold(t->ifp->nd, t->ifp);
 	skb->dev = t->ifp->nd;
 	skb = skb_clone(skb, GFP_ATOMIC);
 	if (skb == NULL) {
-		dev_put(t->ifp->nd);
+		nd_dev_put(t->ifp->nd, t->ifp);
 		return;
 	}
 	f->sent = ktime_get();
@@ -552,7 +552,7 @@ ejectif(struct aoetgt *t, struct aoeif *ifp)
 	n = (e - ifp) * sizeof *ifp;
 	memmove(ifp, ifp+1, n);
 	e->nd = NULL;
-	dev_put(nd);
+	nd_dev_put(nd, NULL);
 }
 
 static struct frame *
@@ -624,7 +624,7 @@ probe(struct aoetgt *t)
 		__skb_queue_tail(&queue, skb);
 		aoenet_xmit(&queue);
 	} else {
-		dev_put(f->t->ifp->nd);
+		nd_dev_put(f->t->ifp->nd, f->t->ifp);
 	}
 }
 
@@ -1403,7 +1403,7 @@ aoecmd_ata_id(struct aoedev *d)
 	ah->cmdstat = ATA_CMD_ID_ATA;
 	ah->lba3 = 0xa0;
 
-	dev_hold(t->ifp->nd);
+	nd_dev_hold(t->ifp->nd, t->ifp);
 	skb->dev = t->ifp->nd;
 
 	d->rttavg = RTTAVG_INIT;
@@ -1414,7 +1414,7 @@ aoecmd_ata_id(struct aoedev *d)
 	if (skb)
 		f->sent = ktime_get();
 	else
-		dev_put(t->ifp->nd);
+		nd_dev_put(t->ifp->nd, t->ifp);
 
 	return skb;
 }
@@ -1514,7 +1514,7 @@ setifbcnt(struct aoetgt *t, struct net_device *nd, int bcnt)
 			pr_err("aoe: device setifbcnt failure; too many interfaces.\n");
 			return;
 		}
-		dev_hold(nd);
+		nd_dev_hold(nd, p);
 		p->nd = nd;
 		p->bcnt = bcnt;
 	}
diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
index 9781488b286b..48c936dbb9e5 100644
--- a/drivers/block/aoe/aoedev.c
+++ b/drivers/block/aoe/aoedev.c
@@ -504,7 +504,8 @@ freetgt(struct aoedev *d, struct aoetgt *t)
 	for (ifp = t->ifs; ifp < &t->ifs[NAOEIFS]; ++ifp) {
 		if (!ifp->nd)
 			break;
-		dev_put(ifp->nd);
+		nd_dev_put(ifp->nd, ifp);
+		aoeif_nd_refcnt_free(ifp);
 	}
 
 	head = &t->ffree;
diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
index 923a134fd766..3565042b567f 100644
--- a/drivers/block/aoe/aoenet.c
+++ b/drivers/block/aoe/aoenet.c
@@ -63,7 +63,7 @@ tx(int id) __must_hold(&txlock)
 			pr_warn("aoe: packet could not be sent on %s.  %s\n",
 				ifp ? ifp->name : "netif",
 				"consider increasing tx_queue_len");
-		dev_put(ifp);
+		nd_dev_put(ifp, NULL);
 		spin_lock_irq(&txlock);
 	}
 	return 0;
-- 
2.35.3
Re: [RFC PATCH 2/2] aoe: using wrappers instead of dev_hold/dev_put for tracking the references of net_device in aoeif
Posted by Greg KH 1 month, 3 weeks ago 
On Wed, Oct 02, 2024 at 12:06:16PM +0800, Chun-Yi Lee wrote:
> Signed-off-by: Chun-Yi Lee <jlee@suse.com>

For obvious reasons, we can't take patches without any changelog
comments, nor really review them either :(

thanks,

greg k-h
Re: [RFC PATCH 2/2] aoe: using wrappers instead of dev_hold/dev_put for tracking the references of net_device in aoeif
Posted by joeyli 1 month, 3 weeks ago 
Hi Greg, 

On Wed, Oct 02, 2024 at 08:30:49AM +0200, Greg KH wrote:
> On Wed, Oct 02, 2024 at 12:06:16PM +0800, Chun-Yi Lee wrote:
> > Signed-off-by: Chun-Yi Lee <jlee@suse.com>
> 
> For obvious reasons, we can't take patches without any changelog
> comments, nor really review them either :(
>

I will merge the 'PATCH 2' with 'PATCH 1' in next version.

Thanks for your review!

Joey Lee
Re: [RFC PATCH 2/2] aoe: using wrappers instead of dev_hold/dev_put for tracking the references of net_device in aoeif
Posted by Damien Le Moal 1 month, 3 weeks ago 
On 10/2/24 1:06 PM, Chun-Yi Lee wrote:
> Signed-off-by: Chun-Yi Lee <jlee@suse.com>

The wrappers where introduced in patch 1 without any user. So it seems that
this patch should be squashed together with patch 1.

> ---
>  drivers/block/aoe/aoecmd.c | 24 ++++++++++++------------
>  drivers/block/aoe/aoedev.c |  3 ++-
>  drivers/block/aoe/aoenet.c |  2 +-
>  3 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
> index d1f4ddc57645..2bae364fc5ef 100644
> --- a/drivers/block/aoe/aoecmd.c
> +++ b/drivers/block/aoe/aoecmd.c
> @@ -361,7 +361,7 @@ ata_rw_frameinit(struct frame *f)
>  	}
>  
>  	ah->cmdstat = ATA_CMD_PIO_READ | writebit | extbit;
> -	dev_hold(t->ifp->nd);
> +	nd_dev_hold(t->ifp->nd, t->ifp);
>  	skb->dev = t->ifp->nd;
>  }
>  
> @@ -403,7 +403,7 @@ aoecmd_ata_rw(struct aoedev *d)
>  		__skb_queue_tail(&queue, skb);
>  		aoenet_xmit(&queue);
>  	} else {
> -		dev_put(f->t->ifp->nd);
> +		nd_dev_put(f->t->ifp->nd, f->t->ifp);
>  	}
>  	return 1;
>  }
> @@ -421,16 +421,16 @@ aoecmd_cfg_pkts(ushort aoemajor, unsigned char aoeminor, struct sk_buff_head *qu
>  
>  	rcu_read_lock();
>  	for_each_netdev_rcu(&init_net, ifp) {
> -		dev_hold(ifp);
> +		nd_dev_hold(ifp, NULL);
>  		if (!is_aoe_netif(ifp)) {
> -			dev_put(ifp);
> +			nd_dev_put(ifp, NULL);
>  			continue;
>  		}
>  
>  		skb = new_skb(sizeof *h + sizeof *ch);
>  		if (skb == NULL) {
>  			printk(KERN_INFO "aoe: skb alloc failure\n");
> -			dev_put(ifp);
> +			nd_dev_put(ifp, NULL);
>  			continue;
>  		}
>  		skb_put(skb, sizeof *h + sizeof *ch);
> @@ -486,11 +486,11 @@ resend(struct aoedev *d, struct frame *f)
>  	memcpy(h->dst, t->addr, sizeof h->dst);
>  	memcpy(h->src, t->ifp->nd->dev_addr, sizeof h->src);
>  
> -	dev_hold(t->ifp->nd);
> +	nd_dev_hold(t->ifp->nd, t->ifp);
>  	skb->dev = t->ifp->nd;
>  	skb = skb_clone(skb, GFP_ATOMIC);
>  	if (skb == NULL) {
> -		dev_put(t->ifp->nd);
> +		nd_dev_put(t->ifp->nd, t->ifp);
>  		return;
>  	}
>  	f->sent = ktime_get();
> @@ -552,7 +552,7 @@ ejectif(struct aoetgt *t, struct aoeif *ifp)
>  	n = (e - ifp) * sizeof *ifp;
>  	memmove(ifp, ifp+1, n);
>  	e->nd = NULL;
> -	dev_put(nd);
> +	nd_dev_put(nd, NULL);
>  }
>  
>  static struct frame *
> @@ -624,7 +624,7 @@ probe(struct aoetgt *t)
>  		__skb_queue_tail(&queue, skb);
>  		aoenet_xmit(&queue);
>  	} else {
> -		dev_put(f->t->ifp->nd);
> +		nd_dev_put(f->t->ifp->nd, f->t->ifp);
>  	}
>  }
>  
> @@ -1403,7 +1403,7 @@ aoecmd_ata_id(struct aoedev *d)
>  	ah->cmdstat = ATA_CMD_ID_ATA;
>  	ah->lba3 = 0xa0;
>  
> -	dev_hold(t->ifp->nd);
> +	nd_dev_hold(t->ifp->nd, t->ifp);
>  	skb->dev = t->ifp->nd;
>  
>  	d->rttavg = RTTAVG_INIT;
> @@ -1414,7 +1414,7 @@ aoecmd_ata_id(struct aoedev *d)
>  	if (skb)
>  		f->sent = ktime_get();
>  	else
> -		dev_put(t->ifp->nd);
> +		nd_dev_put(t->ifp->nd, t->ifp);
>  
>  	return skb;
>  }
> @@ -1514,7 +1514,7 @@ setifbcnt(struct aoetgt *t, struct net_device *nd, int bcnt)
>  			pr_err("aoe: device setifbcnt failure; too many interfaces.\n");
>  			return;
>  		}
> -		dev_hold(nd);
> +		nd_dev_hold(nd, p);
>  		p->nd = nd;
>  		p->bcnt = bcnt;
>  	}
> diff --git a/drivers/block/aoe/aoedev.c b/drivers/block/aoe/aoedev.c
> index 9781488b286b..48c936dbb9e5 100644
> --- a/drivers/block/aoe/aoedev.c
> +++ b/drivers/block/aoe/aoedev.c
> @@ -504,7 +504,8 @@ freetgt(struct aoedev *d, struct aoetgt *t)
>  	for (ifp = t->ifs; ifp < &t->ifs[NAOEIFS]; ++ifp) {
>  		if (!ifp->nd)
>  			break;
> -		dev_put(ifp->nd);
> +		nd_dev_put(ifp->nd, ifp);
> +		aoeif_nd_refcnt_free(ifp);
>  	}
>  
>  	head = &t->ffree;
> diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
> index 923a134fd766..3565042b567f 100644
> --- a/drivers/block/aoe/aoenet.c
> +++ b/drivers/block/aoe/aoenet.c
> @@ -63,7 +63,7 @@ tx(int id) __must_hold(&txlock)
>  			pr_warn("aoe: packet could not be sent on %s.  %s\n",
>  				ifp ? ifp->name : "netif",
>  				"consider increasing tx_queue_len");
> -		dev_put(ifp);
> +		nd_dev_put(ifp, NULL);
>  		spin_lock_irq(&txlock);
>  	}
>  	return 0;


-- 
Damien Le Moal
Western Digital Research
Re: [RFC PATCH 2/2] aoe: using wrappers instead of dev_hold/dev_put for tracking the references of net_device in aoeif
Posted by joeyli 1 month, 3 weeks ago 
Hi Damien,

On Wed, Oct 02, 2024 at 02:37:12PM +0900, Damien Le Moal wrote:
> On 10/2/24 1:06 PM, Chun-Yi Lee wrote:
> > Signed-off-by: Chun-Yi Lee <jlee@suse.com>
> 
> The wrappers where introduced in patch 1 without any user. So it seems that
> this patch should be squashed together with patch 1.
>

I separated this two patches because the second one is base on another patch
'[PATCH v3] aoe: fix the potential use-after-free problem in more places'. 

Now that patch be accepted by Jens:
https://www.spinics.net/lists/kernel/msg5384787.html

According this, I will merge this patch with the 'PATCH 1' in next version.

Thanks for your review and suggestion!

Joey Lee

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.