Bug 200210 - adding vtnet to bridge results to kernel panic
Summary: adding vtnet to bridge results to kernel panic
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-net (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-14 20:47 UTC by Peter Grehan
Modified: 2015-07-01 21:22 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Grehan freebsd_committer 2015-05-14 20:47:02 UTC
Reported by Nikos Vassiliadis on the freebsd-virtualization list,
http://docs.FreeBSD.org/cgi/mid.cgi?55520784.9050508

Unread portion of the kernel message buffer:
panic: if_setflag: decrement non-positive refcount 0 for flag 256
cpuid = 1
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe01a5be54c0
vpanic() at vpanic+0x189/frame 0xfffffe01a5be5540
kassert_panic() at kassert_panic+0x132/frame 0xfffffe01a5be55b0
if_setflag() at if_setflag+0x147/frame 0xfffffe01a5be5610
ifpromisc() at ifpromisc+0x2c/frame 0xfffffe01a5be5640
bridge_delete_member() at bridge_delete_member+0x3a8/frame 0xfffffe01a5be56c0
bridge_ioctl_add() at bridge_ioctl_add+0x574/frame 0xfffffe01a5be5710
bridge_ioctl() at bridge_ioctl+0x1ad/frame 0xfffffe01a5be57b0
ifioctl() at ifioctl+0x9df/frame 0xfffffe01a5be5860
kern_ioctl() at kern_ioctl+0x230/frame 0xfffffe01a5be58c0
sys_ioctl() at sys_ioctl+0x153/frame 0xfffffe01a5be59a0
amd64_syscall() at amd64_syscall+0x27f/frame 0xfffffe01a5be5ab0
Xfast_syscall() at Xfast_syscall+0xfb/frame 0xfffffe01a5be5ab0
--- syscall (54, FreeBSD ELF64, sys_ioctl), rip = 0x8011e09aa, rsp = 0x7fffffffe1f8, rbp = 0x7fffffffe2a0 ---
KDB: enter: panic
Comment 1 commit-hook freebsd_committer 2015-06-13 19:39:36 UTC
A commit references this bug:

Author: kp
Date: Sat Jun 13 19:39:22 UTC 2015
New revision: 284348
URL: https://svnweb.freebsd.org/changeset/base/284348

Log:
  Fix panic when adding vtnet interfaces to a bridge

  vtnet interfaces are always in promiscuous mode (at least if the
  VIRTIO_NET_F_CTRL_RX feature is not negotiated with the host).  if_promisc() on
  a vtnet interface returned ENOTSUP although it has IFF_PROMISC set. This
  confused the bridge code. Instead we now accept all enable/disable promiscuous
  commands (and always keep IFF_PROMISC set).

  There are also two issues with the if_bridge error handling.

  If if_promisc() fails it uses bridge_delete_member() to clean up. This tries to
  disable promiscuous mode on the interface. That runs into an assert, because
  promiscuous mode was never set in the first place. (That's the panic reported in
  PR 200210.)
  We can only unset promiscuous mode if the interface actually is promiscuous.
  This goes against the reference counting done by if_promisc(), but only the
  first/last if_promic() calls can actually fail, so this is safe.

  A second issue is a double free of bif. It's already freed by
  bridge_delete_member().

  PR:		200210
  Differential Revision:	https://reviews.freebsd.org/D2804
  Reviewed by:	philip (mentor)

Changes:
  head/sys/dev/virtio/network/if_vtnet.c
  head/sys/net/if_bridge.c
Comment 2 commit-hook freebsd_committer 2015-07-01 21:22:06 UTC
A commit references this bug:

Author: kp
Date: Wed Jul  1 21:21:15 UTC 2015
New revision: 285016
URL: https://svnweb.freebsd.org/changeset/base/285016

Log:
  MFC r284348: Fix panic when adding vtnet interfaces to a bridge

  vtnet interfaces are always in promiscuous mode (at least if the
  VIRTIO_NET_F_CTRL_RX feature is not negotiated with the host).  if_promisc() on
  a vtnet interface returned ENOTSUP although it has IFF_PROMISC set. This
  confused the bridge code. Instead we now accept all enable/disable promiscuous
  commands (and always keep IFF_PROMISC set).

  There are also two issues with the if_bridge error handling.

  If if_promisc() fails it uses bridge_delete_member() to clean up. This tries to
  disable promiscuous mode on the interface. That runs into an assert, because
  promiscuous mode was never set in the first place. (That's the panic reported in
  PR 200210.)
  We can only unset promiscuous mode if the interface actually is promiscuous.
  This goes against the reference counting done by if_promisc(), but only the
  first/last if_promic() calls can actually fail, so this is safe.

  A second issue is a double free of bif. It's already freed by
  bridge_delete_member().

  PR:         200210

Changes:
_U  stable/10/
  stable/10/sys/dev/virtio/network/if_vtnet.c
  stable/10/sys/net/if_bridge.c