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

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]

Date:   Mon, 19 Aug 2019 15:29:04 +0800
From:   Yang Yingliang <yangyingliang@...wei.com>
To:     Jason Wang <jasowang@...hat.com>, <netdev@...r.kernel.org>
CC:     <eric.dumazet@...il.com>, <xiyou.wangcong@...il.com>,
        <davem@...emloft.net>, <weiyongjun1@...wei.com>
Subject: Re: [PATCH v2] tun: fix use-after-free when register netdev failed



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)
> .
>