Bug 220860 - Double loading of the if_bridge module causes panic
Summary: Double loading of the if_bridge module causes panic
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 11.0-STABLE
Hardware: Any Any
: --- Affects Only Me
Assignee: Kristof Provost
URL:
Keywords: crash
Depends on:
Blocks:
 
Reported: 2017-07-19 18:57 UTC by Vladyslav V. Prodan
Modified: 2017-10-02 12:57 UTC (History)
2 users (show)

See Also:
koobs: mfc-stable11+
koobs: mfc-stable10+


Attachments
Screenshot of the screen at the moment of panic (11.97 KB, image/png)
2017-07-19 18:57 UTC, Vladyslav V. Prodan
no flags Details
Screenshot at the beginning of the panic (10.36 KB, image/png)
2017-07-19 18:59 UTC, Vladyslav V. Prodan
no flags Details
kldstat -v (14.50 KB, text/plain)
2017-07-19 19:01 UTC, Vladyslav V. Prodan
no flags Details
proposed fix (339 bytes, patch)
2017-08-07 08:52 UTC, Eugene Grosbein
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vladyslav V. Prodan 2017-07-19 18:57:27 UTC
Created attachment 184508 [details]
Screenshot of the screen at the moment of panic

Double loading of the if_bridge module causes panic

I have FreeBSD system:
# uname -a
FreeBSD core.domain.com 11.1-PRERELEASE FreeBSD 11.1-PRERELEASE #0 r318606: Mon May 22 14:28:27 EEST 2017     root@core.domain.com:/usr/obj/usr/src/sys/core.domain.com.1  amd64

The if_bridge module is compiled into the kernel

If you re-download it via /boot/loader.conf:
If_bridge_load = "YES"

Then I get a panic and reboot system.

# kldstat -v | grep bridge
                316 bridgestp
                317 if_bridge
                347 ng_bridge

# kldstat
Id Refs Address            Size     Name
 1   15 0xffffffff80200000 13b1b00  kernel
 2    1 0xffffffff815b3000 303178   zfs.ko
 3    2 0xffffffff818b7000 aab0     opensolaris.ko
 4    1 0xffffffff818c2000 29f0     nmdm.ko
 5    1 0xffffffff818c5000 3552e0   vmm.ko
Comment 1 Vladyslav V. Prodan 2017-07-19 18:59:27 UTC
Created attachment 184509 [details]
Screenshot at the beginning of the panic
Comment 2 Vladyslav V. Prodan 2017-07-19 19:01:33 UTC
Created attachment 184510 [details]
kldstat -v
Comment 3 Marie Helene Kvello-Aune 2017-07-28 16:10:10 UTC
I've looked at this problem for about a month and then it kind of rotted on my shelf because busy IRL.

But here's the underlying problems I found which in combination causes this issue:
1) Build system installs kernel modules even if they're built into kernel 
2) boot loader doesn't (successfully) check if module exists in kernel and loads it regardless, as long as it's instructed to do so.

"kldload" properly checks if the module already exists in the kernel and correctly refuses to load the module.

I didn't test all boot loaders but the second problem at least happens with the GPT ZFS loader.
Comment 4 Eugene Grosbein freebsd_committer freebsd_triage 2017-08-07 08:52:50 UTC
Created attachment 185114 [details]
proposed fix

Please try this patch. Apply it and rebuild/reinstall kernel and modules, then repeat the test. It should just complain the module is already present in the kernel and won't panic.
Comment 5 Marie Helene Kvello-Aune 2017-08-07 16:43:16 UTC
Applied patch to 12-current r321460. Issue occured before patch, not after.

With this patch: if_bridge is not loaded when it's compiled into kernel. No error output is printed to dmesg during boot about the module already existing. 

Seems to work as expected now.
Comment 6 commit-hook freebsd_committer freebsd_triage 2017-09-21 14:14:13 UTC
A commit references this bug:

Author: kp
Date: Thu Sep 21 14:14:01 UTC 2017
New revision: 323864
URL: https://svnweb.freebsd.org/changeset/base/323864

Log:
  bridge: Set module version

  This ensures that the loader will not load the module if it's also built in to
  the kernel.

  PR:		220860
  Submitted by:	Eugene Grosbein <eugen@freebsd.org>
  Reported by:	Marie Helene Kvello-Aune <marieheleneka@gmail.com>

Changes:
  head/sys/net/if_bridge.c
Comment 7 Marie Helene Kvello-Aune 2017-09-21 14:44:39 UTC
I should probably point out the error in the commit message as I didn't submit the PR. But I did discuss it with kp at length in June. ;)
Comment 8 commit-hook freebsd_committer freebsd_triage 2017-09-30 10:15:40 UTC
A commit references this bug:

Author: kp
Date: Sat Sep 30 10:15:04 UTC 2017
New revision: 324115
URL: https://svnweb.freebsd.org/changeset/base/324115

Log:
  MFC r323864

  bridge: Set module version

  This ensures that the loader will not load the module if it's also built in to
  the kernel.

  PR:             220860
  Submitted by:   Eugene Grosbein <eugen@freebsd.org>

Changes:
_U  stable/11/
  stable/11/sys/net/if_bridge.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-09-30 10:16:46 UTC
A commit references this bug:

Author: kp
Date: Sat Sep 30 10:16:15 UTC 2017
New revision: 324116
URL: https://svnweb.freebsd.org/changeset/base/324116

Log:
  MFC r323864

  bridge: Set module version

  This ensures that the loader will not load the module if it's also built in to
  the kernel.

  PR:             220860
  Submitted by:   Eugene Grosbein <eugen@freebsd.org>

Changes:
_U  stable/10/
  stable/10/sys/net/if_bridge.c
Comment 10 Kubilay Kocak freebsd_committer freebsd_triage 2017-10-02 11:49:17 UTC
Thank you for resolving this (and the merges to both stable branches in particular).