Subject: Re: [PATCH v2] tun: fix use-after-free when register netdev failed

From: Yang Yingliang <[email-masked]>
Date: Mon, 19 Aug 2019 15:29:04 +0800

On 2019/8/19 11:17, Jason Wang wrote:
> On 2019/8/16 下午7:00, Yang Yingliang wrote:
[...]
>>   
>>   		INIT_LIST_HEAD(&tun->disabled);
>> -		err = tun_attach(tun, file, false, ifr->ifr_flags & IFF_NAPI,
>> -				 ifr->ifr_flags & IFF_NAPI_FRAGS);
>> -		if (err < 0)
>> -			goto err_free_flow;
>> +
>> +		tun_set_real_num_queues(tun, tun->numqueues + 1);
>
> This looks tricky, why not simply call netif_set_real_num_tx/rx_queues()
> here?
OK, I will do some test, then send a v3 patch.

Thanks,
Yang

>
> Thanks
>
>
>>   
>>   		err = register_netdevice(tun->dev);
>>   		if (err < 0)
>> -			goto err_detach;
>> +			/* register_netdevice() already called tun_free_netdev() */
>> +			goto err_free_dev;
>> +
>> +		err = tun_attach(tun, file, false, ifr->ifr_flags & IFF_NAPI,
>> +				 ifr->ifr_flags & IFF_NAPI_FRAGS);
>> +		if (err < 0)
>> +			goto err_unregister;
>>   	}
>>   
>>   	netif_carrier_on(tun->dev);
>> @@ -2851,14 +2857,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>>   	strcpy(ifr->ifr_name, tun->dev->name);
>>   	return 0;
>>   
>> -err_detach:
>> -	tun_detach_all(dev);
>> -	/* register_netdevice() already called tun_free_netdev() */
>> -	goto err_free_dev;
>> +err_unregister:
>> +	unregister_netdevice(dev);
>> +	return err;
>>   
>> -err_free_flow:
>> -	tun_flow_uninit(tun);
>> -	security_tun_dev_free_security(tun->security);
>>   err_free_stat:
>>   	free_percpu(tun->pcpu_stats);
>>   err_free_dev:
>> @@ -2979,6 +2981,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>   			goto unlock;
>>   		ret = tun_attach(tun, file, false, tun->flags & IFF_NAPI,
>>   				 tun->flags & IFF_NAPI_FRAGS);
>> +		if (!ret)
>> +			tun_set_real_num_queues(tun, tun->numqueues);
>>   	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>>   		tun = rtnl_dereference(tfile->tun);
>>   		if (!tun || !(tun->flags & IFF_MULTI_QUEUE) || tfile->detached)
> .
>