netdev - Re: The ubufs->refcount maybe be subtracted twice when tun_get

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <fad5d47a-96ee-1afc-b10c-cb2a6d5dc784@redhat.com>
Date:   Wed, 30 Nov 2016 10:53:47 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     wangyunjian <wangyunjian@...wei.com>,
        "mst@...hat.com" <mst@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc:     caihe <caihe@...wei.com>
Subject: Re: The ubufs->refcount maybe be subtracted twice when tun_get_user
 failed



On 2016年11月29日 21:27, wangyunjian wrote:
> Sorry, I didn't describe it clearly. In fact, the second subtraction happens in the function handle_tx,
> when tun_get_user fails and zcopy_used is ture. Fllowing the steps:

I get your meaning. Thanks for the reporting. Will post patches (since 
macvtap has similar issue).


>
> static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>                                    void *msg_control, struct iov_iter *from,
>                                    int noblock) {
>             ...
>
>             if (zerocopy)
>                       err = zerocopy_sg_from_iter(skb, from);
>             else {
>                       err = skb_copy_datagram_from_iter(skb, 0, from, len);
>                       if (!err && msg_control) {
>                                struct ubuf_info *uarg = msg_control;
>                                uarg->callback(uarg, false);                       --> step 1, the ubufs->refcount is subtracted frist.
>                       }
>             }
>
>             if (err) {
>                       this_cpu_inc(tun->pcpu_stats->rx_dropped);
>                       kfree_skb(skb);
>                       return -EFAULT;
>             }
>
>             err = virtio_net_hdr_to_skb(skb, &gso, tun_is_little_endian(tun));
>             if (err) {
>                       this_cpu_inc(tun->pcpu_stats->rx_frame_errors);
>                       kfree_skb(skb);
>                       return -EINVAL;                                         -->step 2, return err.
>             }
> }
>
> static void handle_tx(struct vhost_net *net)
> {
> 	...
> 	/* TODO: Check specific error and bomb out unless ENOBUFS? */
> 	err = sock->ops->sendmsg(sock, &msg, len);
> 	if (unlikely(err < 0)) {
> 		if (zcopy_used) {
> 			vhost_net_ubuf_put(ubufs);                                        --> step 3, the ubufs->refcount will be subtracted twice, when sendmsg execution err.
> 			nvq->upend_idx = ((unsigned)nvq->upend_idx - 1)
> 				% UIO_MAXIOV;
> 		}
> 		vhost_discard_vq_desc(vq, 1);
> 		break;
> 	}
> 	...
> }
>
> -----Original Message-----