From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Gora Subject: [PATCH v2 03/10] kni: don't touch struct kni_dev after freeing Date: Thu, 28 Jun 2018 18:55:01 -0700 Message-ID: <20180629015508.26599-4-dg@adax.com> References: <20180628224513.18391-1-dg@adax.com> <20180629015508.26599-1-dg@adax.com> Cc: dev@dpdk.org, Dan Gora To: ferruh.yigit@intel.com Return-path: Received: from mail-oi0-f65.google.com (mail-oi0-f65.google.com [209.85.218.65]) by dpdk.org (Postfix) with ESMTP id 7402A1B532 for ; Fri, 29 Jun 2018 03:55:52 +0200 (CEST) Received: by mail-oi0-f65.google.com with SMTP id k81-v6so7044861oib.4 for ; Thu, 28 Jun 2018 18:55:52 -0700 (PDT) In-Reply-To: <20180629015508.26599-1-dg@adax.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Since the struct kni_dev is allocated as part of the netdev with alloc_netdev, when free_netdev is called, this also frees the struct kni_dev embedded in it. This means that we cannot touch struct kni_dev after calling free_netdev since that memory was already deallocated. Separate unregistering the net_dev with unregister_netdev from freeing the net_device and kni_dev structures into separate functions and ensure that we do not touch anything in the kni_dev structure after freeing it (ie don't call list_del(), etc...). Signed-off-by: Dan Gora --- kernel/linux/kni/kni_dev.h | 1 + kernel/linux/kni/kni_misc.c | 28 +++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index ed42989cc..f9aa90ff9 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -37,6 +37,7 @@ struct kni_dev { struct list_head list; struct net_device_stats stats; + uint16_t registered; /* 0 if already released, 1 otherwise */ uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ char name[RTE_KNI_NAMESIZE]; /* Network device name */ diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index d889ffc4b..1c38cfa1a 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -184,17 +184,34 @@ kni_dev_remove(struct kni_dev *dev) ixgbe_kni_remove(dev->pci_dev); else if (pci_match_id(igb_pci_tbl, dev->pci_dev)) igb_kni_remove(dev->pci_dev); + dev->pci_dev = NULL; } #endif - if (dev->net_dev) { + if (dev->registered) { unregister_netdev(dev->net_dev); - free_netdev(dev->net_dev); + dev->registered = 0; } return 0; } +static void +kni_dev_free(struct kni_dev *dev) +{ + struct net_device *net_dev; + /* + * Remember that struct kni_dev is part of the netdev + * structure, so we free *both* with free_netdev. + */ + if (dev == NULL) + return; + net_dev = dev->net_dev; + dev->net_dev = NULL; + if (net_dev) + free_netdev(net_dev); +} + static int kni_release(struct inode *inode, struct file *file) { @@ -224,6 +241,7 @@ kni_release(struct inode *inode, struct file *file) kni_dev_remove(dev); kni_net_release_fifo_phy(dev); list_del(&dev->list); + kni_dev_free(dev); } up_write(&knet->kni_list_lock); @@ -457,15 +475,18 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, if (ret) { pr_err("error %i registering device \"%s\"\n", ret, dev_info.name); + kni_dev_remove(kni); kni_net_release_fifo_phy(kni); - free_netdev(net_dev); + kni_dev_free(kni); return -ENODEV; } + kni->registered = 1; ret = kni_run_thread(knet, kni, dev_info.force_bind); if (ret != 0) { kni_dev_remove(kni); kni_net_release_fifo_phy(kni); + kni_dev_free(kni); return ret; } @@ -550,6 +571,7 @@ kni_ioctl_free(struct net *net, uint32_t ioctl_num, kni_net_release_fifo_phy(dev); list_del(&dev->list); + kni_dev_free(dev); up_write(&knet->kni_list_lock); pr_info("Successfully freed kni interface: %s\n", dev_info.name); -- 2.18.0.rc1.1.g6f333ff2f