[dpdk-dev] [PATCH] kni: unregister an unregisterd net_device could cause a kernel crash

[Ferruh Yigit]

On 9/9/2016 3:46 AM, zhouyangchao wrote:
> Signed-off-by: zhouyangchao <zhouyates at gmail.com>
> ---
>  lib/librte_eal/linuxapp/kni/kni_misc.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
> index 67e9b7d..ad4e603 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> @@ -361,7 +361,9 @@ kni_dev_remove(struct kni_dev *dev)
>  		igb_kni_remove(dev->pci_dev);
>  
>  	if (dev->net_dev) {
> -		unregister_netdev(dev->net_dev);
> +		if (dev->netdev->reg_state == NETREG_REGISTERED){

Although this will work, I believe we shouldn't know more about netdev
internals unless we don't have to, and for this case we don't need to
know it. More Linux internal knowledge means more ways to be broken in
the future.
Also I am not sure if reg_state intended to be used by network drivers.

I am for first version of your patch, with missing free_netdev() added
into rollback code.

pseudo code:
net_dev = alloc_netdev()
...
ret = register_netdev()
if (ret)
	kni_dev_remove()
	free_netdev()
	return
kni->net_dev = net_dev


OR if don't want to move where kni->net_dev assigned

net_dev = alloc_netdev()
kni->net_dev = net_dev
...
ret = register_netdev()
if (ret)
	kni->net_dev = NULL
	kni_dev_remove()
	free_netdev()
	return




> +			unregister_netdev(dev->net_dev);
> +		}
>  		free_netdev(dev->net_dev);
>  	}
>  
>