Re: The ubufs->refcount maybe be subtracted twice when tun_get_user failed

[Jason Wang]

On 2016年11月29日 17:30, wangyunjian wrote:
In function tun_get_user , the ubufs->refcount may be subtracted twice, when 
msg_control is true and zerocopy is false.

About the below code:

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);                       
--> 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;                                   -->here, the 
ubufs->refcount will be subtracted twice, when virtio_net_hdr_to_skb execution err.
          }

Just make sure I understand the problem. Since we don't set 
SKBTX_DEV_ZEROCOPY here if zerocopy is false, callback won't be even 
trigged in skb_release_data(). So we are in fact safe here?

Thanks

switch (tun->flags & TUN_TYPE_MASK) {
          case IFF_TUN:
                    if (tun->flags & IFF_NO_PI) {
                             switch (skb->data[0] & 0xf0) {
                             case 0x40:
                                      pi.proto = htons(ETH_P_IP);
                                      break;
                             case 0x60:
                                      pi.proto = htons(ETH_P_IPV6);
                                      break;
                             default:
                                      this_cpu_inc(tun->pcpu_stats->rx_dropped);
                                      kfree_skb(skb);
                                      return -EINVAL;                          
--> this will also be subtracted twice.
                             }
                    }

                    skb_reset_mac_header(skb);
                    skb->protocol = pi.proto;
                    skb->dev = tun->dev;
                    break;
          case IFF_TAP:
                    skb->protocol = eth_type_trans(skb, tun->dev);
                    break;
          }
                ...
}