Skip to content
Snippets Groups Projects
  1. Aug 21, 2020
    • Andrea Righi's avatar
      xen-netfront: fix potential deadlock in xennet_remove() · 6015a692
      Andrea Righi authored
      
      [ Upstream commit c2c63310 ]
      
      There's a potential race in xennet_remove(); this is what the driver is
      doing upon unregistering a network device:
      
        1. state = read bus state
        2. if state is not "Closed":
        3.    request to set state to "Closing"
        4.    wait for state to be set to "Closing"
        5.    request to set state to "Closed"
        6.    wait for state to be set to "Closed"
      
      If the state changes to "Closed" immediately after step 1 we are stuck
      forever in step 4, because the state will never go back from "Closed" to
      "Closing".
      
      Make sure to check also for state == "Closed" in step 4 to prevent the
      deadlock.
      
      Also add a 5 sec timeout any time we wait for the bus state to change,
      to avoid getting stuck forever in wait_event().
      
      Signed-off-by: default avatarAndrea Righi <andrea.righi@canonical.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      6015a692
  2. Oct 07, 2019
    • Dongli Zhang's avatar
      xen-netfront: do not use ~0U as error return value for xennet_fill_frags() · d51c3f70
      Dongli Zhang authored
      
      [ Upstream commit a761129e ]
      
      xennet_fill_frags() uses ~0U as return value when the sk_buff is not able
      to cache extra fragments. This is incorrect because the return type of
      xennet_fill_frags() is RING_IDX and 0xffffffff is an expected value for
      ring buffer index.
      
      In the situation when the rsp_cons is approaching 0xffffffff, the return
      value of xennet_fill_frags() may become 0xffffffff which xennet_poll() (the
      caller) would regard as error. As a result, queue->rx.rsp_cons is set
      incorrectly because it is updated only when there is error. If there is no
      error, xennet_poll() would be responsible to update queue->rx.rsp_cons.
      Finally, queue->rx.rsp_cons would point to the rx ring buffer entries whose
      queue->rx_skbs[i] and queue->grant_rx_ref[i] are already cleared to NULL.
      This leads to NULL pointer access in the next iteration to process rx ring
      buffer entries.
      
      The symptom is similar to the one fixed in
      commit 00b36850 ("xen-netfront: do not assume sk_buff_head list is
      empty in error handling").
      
      This patch changes the return type of xennet_fill_frags() to indicate
      whether it is successful or failed. The queue->rx.rsp_cons will be
      always updated inside this function.
      
      Fixes: ad4f15dc ("xen/netfront: don't bug in case of too many frags")
      Signed-off-by: default avatarDongli Zhang <dongli.zhang@oracle.com>
      Reviewed-by: default avatarJuergen Gross <jgross@suse.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      d51c3f70
  3. Sep 21, 2019
    • Dongli Zhang's avatar
      xen-netfront: do not assume sk_buff_head list is empty in error handling · 08ea4ff3
      Dongli Zhang authored
      
      [ Upstream commit 00b36850 ]
      
      When skb_shinfo(skb) is not able to cache extra fragment (that is,
      skb_shinfo(skb)->nr_frags >= MAX_SKB_FRAGS), xennet_fill_frags() assumes
      the sk_buff_head list is already empty. As a result, cons is increased only
      by 1 and returns to error handling path in xennet_poll().
      
      However, if the sk_buff_head list is not empty, queue->rx.rsp_cons may be
      set incorrectly. That is, queue->rx.rsp_cons would point to the rx ring
      buffer entries whose queue->rx_skbs[i] and queue->grant_rx_ref[i] are
      already cleared to NULL. This leads to NULL pointer access in the next
      iteration to process rx ring buffer entries.
      
      Below is how xennet_poll() does error handling. All remaining entries in
      tmpq are accounted to queue->rx.rsp_cons without assuming how many
      outstanding skbs are remained in the list.
      
       985 static int xennet_poll(struct napi_struct *napi, int budget)
      ... ...
      1032           if (unlikely(xennet_set_skb_gso(skb, gso))) {
      1033                   __skb_queue_head(&tmpq, skb);
      1034                   queue->rx.rsp_cons += skb_queue_len(&tmpq);
      1035                   goto err;
      1036           }
      
      It is better to always have the error handling in the same way.
      
      Fixes: ad4f15dc ("xen/netfront: don't bug in case of too many frags")
      Signed-off-by: default avatarDongli Zhang <dongli.zhang@oracle.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      08ea4ff3
  4. Jan 09, 2019
  5. Nov 10, 2018
  6. Sep 29, 2018
  7. Sep 26, 2018
    • Juergen Gross's avatar
      xen/netfront: fix waiting for xenbus state change · 03ae5ff3
      Juergen Gross authored
      
      commit 8edfe2e9 upstream.
      
      Commit 822fb18a ("xen-netfront: wait xenbus state change when load
      module manually") added a new wait queue to wait on for a state change
      when the module is loaded manually. Unfortunately there is no wakeup
      anywhere to stop that waiting.
      
      Instead of introducing a new wait queue rename the existing
      module_unload_q to module_wq and use it for both purposes (loading and
      unloading).
      
      As any state change of the backend might be intended to stop waiting
      do the wake_up_all() in any case when netback_changed() is called.
      
      Fixes: 822fb18a ("xen-netfront: wait xenbus state change when load module manually")
      Cc: <stable@vger.kernel.org> #4.18
      Signed-off-by: default avatarJuergen Gross <jgross@suse.com>
      Reviewed-by: default avatarBoris Ostrovsky <boris.ostrovsky@oracle.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      03ae5ff3
    • Xiao Liang's avatar
      xen-netfront: fix warn message as irq device name has '/' · 9d47d030
      Xiao Liang authored
      
      [ Upstream commit 21f2706b ]
      
      There is a call trace generated after commit 2d408c0d(
      xen-netfront: fix queue name setting). There is no 'device/vif/xx-q0-tx' file found
      under /proc/irq/xx/.
      
      This patch only picks up device type and id as its name.
      
      With the patch, now /proc/interrupts looks like below and the warning message gone:
       70:         21          0          0          0   xen-dyn    -event     vif0-q0-tx
       71:         15          0          0          0   xen-dyn    -event     vif0-q0-rx
       72:         14          0          0          0   xen-dyn    -event     vif0-q1-tx
       73:         33          0          0          0   xen-dyn    -event     vif0-q1-rx
       74:         12          0          0          0   xen-dyn    -event     vif0-q2-tx
       75:         24          0          0          0   xen-dyn    -event     vif0-q2-rx
       76:         19          0          0          0   xen-dyn    -event     vif0-q3-tx
       77:         21          0          0          0   xen-dyn    -event     vif0-q3-rx
      
      Below is call trace information without this patch:
      
      name 'device/vif/0-q0-tx'
      WARNING: CPU: 2 PID: 37 at fs/proc/generic.c:174 __xlate_proc_name+0x85/0xa0
      RIP: 0010:__xlate_proc_name+0x85/0xa0
      RSP: 0018:ffffb85c40473c18 EFLAGS: 00010286
      RAX: 0000000000000000 RBX: 0000000000000006 RCX: 0000000000000006
      RDX: 0000000000000007 RSI: 0000000000000096 RDI: ffff984c7f516930
      RBP: ffffb85c40473cb8 R08: 000000000000002c R09: 0000000000000229
      R10: 0000000000000000 R11: 0000000000000001 R12: ffffb85c40473c98
      R13: ffffb85c40473cb8 R14: ffffb85c40473c50 R15: 0000000000000000
      FS:  0000000000000000(0000) GS:ffff984c7f500000(0000) knlGS:0000000000000000
      CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      CR2: 00007f69b6899038 CR3: 000000001c20a006 CR4: 00000000001606e0
      Call Trace:
      __proc_create+0x45/0x230
      ? snprintf+0x49/0x60
      proc_mkdir_data+0x35/0x90
      register_handler_proc+0xef/0x110
      ? proc_register+0xfc/0x110
      ? proc_create_data+0x70/0xb0
      __setup_irq+0x39b/0x660
      ? request_threaded_irq+0xad/0x160
      request_threaded_irq+0xf5/0x160
      ? xennet_tx_buf_gc+0x1d0/0x1d0 [xen_netfront]
      bind_evtchn_to_irqhandler+0x3d/0x70
      ? xenbus_alloc_evtchn+0x41/0xa0
      netback_changed+0xa46/0xcda [xen_netfront]
      ? find_watch+0x40/0x40
      xenwatch_thread+0xc5/0x160
      ? finish_wait+0x80/0x80
      kthread+0x112/0x130
      ? kthread_create_worker_on_cpu+0x70/0x70
      ret_from_fork+0x35/0x40
      Code: 81 5c 00 48 85 c0 75 cc 5b 49 89 2e 31 c0 5d 4d 89 3c 24 41 5c 41 5d 41 5e 41 5f c3 4c 89 ee 48 c7 c7 40 4f 0e b4 e8 65 ea d8 ff <0f> 0b b8 fe ff ff ff 5b 5d 41 5c 41 5d 41 5e 41 5f c3 66 0f 1f
      ---[ end trace 650e5561b0caab3a ]---
      
      Signed-off-by: default avatarXiao Liang <xiliang@redhat.com>
      Reviewed-by: default avatarJuergen Gross <jgross@suse.com>
      
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      
      Signed-off-by: default avatarSasha Levin <alexander.levin@microsoft.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      9d47d030
    • Vitaly Kuznetsov's avatar
      xen-netfront: fix queue name setting · 21fa7488
      Vitaly Kuznetsov authored
      
      [ Upstream commit 2d408c0d ]
      
      Commit f599c64f ("xen-netfront: Fix race between device setup and
      open") changed the initialization order: xennet_create_queues() now
      happens before we do register_netdev() so using netdev->name in
      xennet_init_queue() is incorrect, we end up with the following in
      /proc/interrupts:
      
       60:        139          0   xen-dyn    -event     eth%d-q0-tx
       61:        265          0   xen-dyn    -event     eth%d-q0-rx
       62:        234          0   xen-dyn    -event     eth%d-q1-tx
       63:          1          0   xen-dyn    -event     eth%d-q1-rx
      
      and this looks ugly. Actually, using early netdev name (even when it's
      already set) is also not ideal: nowadays we tend to rename eth devices
      and queue name may end up not corresponding to the netdev name.
      
      Use nodename from xenbus device for queue naming: this can't change in VM's
      lifetime. Now /proc/interrupts looks like
      
       62:        202          0   xen-dyn    -event     device/vif/0-q0-tx
       63:        317          0   xen-dyn    -event     device/vif/0-q0-rx
       64:        262          0   xen-dyn    -event     device/vif/0-q1-tx
       65:         17          0   xen-dyn    -event     device/vif/0-q1-rx
      
      Fixes: f599c64f ("xen-netfront: Fix race between device setup and open")
      Signed-off-by: default avatarVitaly Kuznetsov <vkuznets@redhat.com>
      Reviewed-by: default avatarRoss Lagerwall <ross.lagerwall@citrix.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarSasha Levin <alexander.levin@microsoft.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      21fa7488
  8. Aug 15, 2018
  9. Aug 06, 2018
    • Xiao Liang's avatar
      xen-netfront: wait xenbus state change when load module manually · b03ca669
      Xiao Liang authored
      
      [ Upstream commit 822fb18a ]
      
      When loading module manually, after call xenbus_switch_state to initializes
      the state of the netfront device, the driver state did not change so fast
      that may lead no dev created in latest kernel. This patch adds wait to make
      sure xenbus knows the driver is not in closed/unknown state.
      
      Current state:
      [vm]# ethtool eth0
      Settings for eth0:
      	Link detected: yes
      [vm]# modprobe -r xen_netfront
      [vm]# modprobe  xen_netfront
      [vm]# ethtool eth0
      Settings for eth0:
      Cannot get device settings: No such device
      Cannot get wake-on-lan settings: No such device
      Cannot get message level: No such device
      Cannot get link status: No such device
      No data available
      
      With the patch installed.
      [vm]# ethtool eth0
      Settings for eth0:
      	Link detected: yes
      [vm]# modprobe -r xen_netfront
      [vm]# modprobe xen_netfront
      [vm]# ethtool eth0
      Settings for eth0:
      	Link detected: yes
      
      Signed-off-by: default avatarXiao Liang <xiliang@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      b03ca669
  10. Aug 03, 2018
  11. May 30, 2018
    • Ross Lagerwall's avatar
      xen-netfront: Fix race between device setup and open · ca3108cd
      Ross Lagerwall authored
      
      [ Upstream commit f599c64f ]
      
      When a netfront device is set up it registers a netdev fairly early on,
      before it has set up the queues and is actually usable. A userspace tool
      like NetworkManager will immediately try to open it and access its state
      as soon as it appears. The bug can be reproduced by hotplugging VIFs
      until the VM runs out of grant refs. It registers the netdev but fails
      to set up any queues (since there are no more grant refs). In the
      meantime, NetworkManager opens the device and the kernel crashes trying
      to access the queues (of which there are none).
      
      Fix this in two ways:
      * For initial setup, register the netdev much later, after the queues
      are setup. This avoids the race entirely.
      * During a suspend/resume cycle, the frontend reconnects to the backend
      and the queues are recreated. It is possible (though highly unlikely) to
      race with something opening the device and accessing the queues after
      they have been destroyed but before they have been recreated. Extend the
      region covered by the rtnl semaphore to protect against this race. There
      is a possibility that we fail to recreate the queues so check for this
      in the open function.
      
      Signed-off-by: default avatarRoss Lagerwall <ross.lagerwall@citrix.com>
      Reviewed-by: default avatarBoris Ostrovsky <boris.ostrovsky@oracle.com>
      Signed-off-by: default avatarJuergen Gross <jgross@suse.com>
      Signed-off-by: default avatarSasha Levin <alexander.levin@microsoft.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      ca3108cd
  12. Apr 24, 2018
  13. Mar 03, 2018
  14. Feb 03, 2018
    • Eduardo Otubo's avatar
      xen-netfront: remove warning when unloading module · c6a34556
      Eduardo Otubo authored
      
      
      [ Upstream commit 5b5971df ]
      
      v2:
       * Replace busy wait with wait_event()/wake_up_all()
       * Cannot garantee that at the time xennet_remove is called, the
         xen_netback state will not be XenbusStateClosed, so added a
         condition for that
       * There's a small chance for the xen_netback state is
         XenbusStateUnknown by the time the xen_netfront switches to Closed,
         so added a condition for that.
      
      When unloading module xen_netfront from guest, dmesg would output
      warning messages like below:
      
        [  105.236836] xen:grant_table: WARNING: g.e. 0x903 still in use!
        [  105.236839] deferring g.e. 0x903 (pfn 0x35805)
      
      This problem relies on netfront and netback being out of sync. By the time
      netfront revokes the g.e.'s netback didn't have enough time to free all of
      them, hence displaying the warnings on dmesg.
      
      The trick here is to make netfront to wait until netback frees all the g.e.'s
      and only then continue to cleanup for the module removal, and this is done by
      manipulating both device states.
      
      Signed-off-by: default avatarEduardo Otubo <otubo@redhat.com>
      Acked-by: default avatarJuergen Gross <jgross@suse.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarSasha Levin <alexander.levin@microsoft.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      c6a34556
  15. Dec 09, 2017
  16. Jul 21, 2017
  17. Jul 05, 2017
  18. Feb 14, 2017
  19. Nov 02, 2016
  20. Oct 31, 2016
  21. Sep 20, 2016
    • Vitaly Kuznetsov's avatar
      xen-netfront: avoid packet loss when ethernet header crosses page boundary · fd07160b
      Vitaly Kuznetsov authored
      
      Small packet loss is reported on complex multi host network configurations
      including tunnels, NAT, ... My investigation led me to the following check
      in netback which drops packets:
      
              if (unlikely(txreq.size < ETH_HLEN)) {
                      netdev_err(queue->vif->dev,
                                 "Bad packet size: %d\n", txreq.size);
                      xenvif_tx_err(queue, &txreq, extra_count, idx);
                      break;
              }
      
      But this check itself is legitimate. SKBs consist of a linear part (which
      has to have the ethernet header) and (optionally) a number of frags.
      Netfront transmits the head of the linear part up to the page boundary
      as the first request and all the rest becomes frags so when we're
      reconstructing the SKB in netback we can't distinguish between original
      frags and the 'tail' of the linear part. The first SKB needs to be at
      least ETH_HLEN size. So in case we have an SKB with its linear part
      starting too close to the page boundary the packet is lost.
      
      I see two ways to fix the issue:
      - Change the 'wire' protocol between netfront and netback to start keeping
        the original SKB structure. We'll have to add a flag indicating the fact
        that the particular request is a part of the original linear part and not
        a frag. We'll need to know the length of the linear part to pre-allocate
        memory.
      - Avoid transmitting SKBs with linear parts starting too close to the page
        boundary. That seems preferable short-term and shouldn't bring
        significant performance degradation as such packets are rare. That's what
        this patch is trying to achieve with skb_copy().
      
      Signed-off-by: default avatarVitaly Kuznetsov <vkuznets@redhat.com>
      Acked-by: default avatarDavid Vrabel <david.vrabel@citrix.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      fd07160b
  22. Jan 29, 2016
  23. Oct 23, 2015
  24. Oct 21, 2015
  25. Sep 21, 2015
  26. Sep 10, 2015
  27. Sep 08, 2015
  28. Aug 28, 2015
    • Chas Williams's avatar
      net/xen-netfront: only napi_synchronize() if running · 274b0455
      Chas Williams authored
      
      If an interface isn't running napi_synchronize() will hang forever.
      
      [  392.248403] rmmod           R  running task        0   359    343 0x00000000
      [  392.257671]  ffff88003760fc88 ffff880037193b40 ffff880037193160 ffff88003760fc88
      [  392.267644]  ffff880037610000 ffff88003760fcd8 0000000100014c22 ffffffff81f75c40
      [  392.277524]  0000000000bc7010 ffff88003760fca8 ffffffff81796927 ffffffff81f75c40
      [  392.287323] Call Trace:
      [  392.291599]  [<ffffffff81796927>] schedule+0x37/0x90
      [  392.298553]  [<ffffffff8179985b>] schedule_timeout+0x14b/0x280
      [  392.306421]  [<ffffffff810f91b9>] ? irq_free_descs+0x69/0x80
      [  392.314006]  [<ffffffff811084d0>] ? internal_add_timer+0xb0/0xb0
      [  392.322125]  [<ffffffff81109d07>] msleep+0x37/0x50
      [  392.329037]  [<ffffffffa00ec79a>] xennet_disconnect_backend.isra.24+0xda/0x390 [xen_netfront]
      [  392.339658]  [<ffffffffa00ecadc>] xennet_remove+0x2c/0x80 [xen_netfront]
      [  392.348516]  [<ffffffff81481c69>] xenbus_dev_remove+0x59/0xc0
      [  392.356257]  [<ffffffff814e7217>] __device_release_driver+0x87/0x120
      [  392.364645]  [<ffffffff814e7cf8>] driver_detach+0xb8/0xc0
      [  392.371989]  [<ffffffff814e6e69>] bus_remove_driver+0x59/0xe0
      [  392.379883]  [<ffffffff814e84f0>] driver_unregister+0x30/0x70
      [  392.387495]  [<ffffffff814814b2>] xenbus_unregister_driver+0x12/0x20
      [  392.395908]  [<ffffffffa00ed89b>] netif_exit+0x10/0x775 [xen_netfront]
      [  392.404877]  [<ffffffff81124e08>] SyS_delete_module+0x1d8/0x230
      [  392.412804]  [<ffffffff8179a8ee>] system_call_fastpath+0x12/0x71
      
      Signed-off-by: default avatarChas Williams <3chas3@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      274b0455
  29. Aug 23, 2015
  30. Jun 28, 2015
  31. Jun 21, 2015
  32. Jun 17, 2015
  33. Jun 01, 2015
  34. May 27, 2015
  35. Apr 17, 2015
  36. Apr 15, 2015
Loading