The bad: label in the previous revision gets called for ether types 0x88e1 (HomePlug AV management frames) and 0x8912 (not named in sys/net/ethernet.h) via the default: case and with net.link.bridge.pfil_onlyip=1. This looks to me like a policy drop and not an error path. As such I removed that part. Correct me if I get this wrong... (447 such packets in 300s doesn't not sound like an error to me).

rework: the bad: counter is removed; only the fragmentation path is counted

The following part may need to get a rework for this change... investigating...

In D59332#1362525, @markj wrote:

There are a couple of other places in if_bridge where we m_copyup() up to max_protohdr bytes. Do we need to fix those as well? m_copyup() doesn't handle extpg mbufs, though it wouldn't be hard to just fix that directly.

In D59389#1362863, @glebius wrote:

Ok, this mess mostly comes from me. While reviewing this patch I came to alternative that I will post soon.

D59389 has a similar fix for ipfw.

Unit test for the issue in D59389.

Narrowed per markj@: pull up ETHER_HDR_LEN first, the SNAP/LLC header only when the frame is long enough to carry one. No mb_unmapped_to_ext() call. Rebased on current main.

In D59332#1361528, @netchild wrote:

After a bit more digging around. This does not seem only to be a particular problem for the bridge, this is a problem for everything pfil related...
ipfw_check_frame_mbuf() seems to have the same bug (depends on net.link.ether.ipfw=1).
pf's ethernet hook seems to be safe at first look (m_copydata()).
if_enc passes the chain without touching, safe.
dummynet: pulls 14 bytes, so probably safe.
ipfilter: no idea, maybe.

This is a test for the fix in D59332

Unit tests in D59333

In D59204#1360863, @gallatin wrote:

In D59204#1360536, @netchild wrote:

The patchset is now running on the host which needs the watchdog to trigger from time to time. It needs a while to run into the issue.

Any idea how long this will take to trigger? I'd really like this patch to be pushed, as my test machines are all delivering spurious WD resets and need this patch merged (and a follow-on of my own to fix the world for ktls as well)

This fixes the hang at boot I've seen.

But don't we want the second one to fail?
Why do I want the second one to succeed?
When I run a non-svc start in parllel, I would want only one to succeed (if we think about nginx, or mysql or such).
What if someone hits ctrl-c after taking the lock?

The patchset is now running on the host which needs the watchdog to trigger from time to time. It needs a while to run into the issue.

My build is still running on the old hardware where I can test this.
Quick check, if I get it right:

• this solves the issue where demand is false on the next period.
• this does not solve an issue where ~1 packet per period is send while more than one is in the queue (no idea if this is behavior we can see in the real world): growth every period → demand true every period → armed reaches 4 → still resets.

In D59102#1356034, @markj wrote:

In D59102#1355583, @netchild wrote:

	if (PMC_SYSTEM_SAMPLING_ACTIVE() || PMC_PROC_IS_USING_PMCS(p)) {
		VOP_UNLOCK(imgp->vp);
-		pe.pm_credentialschanged = credential_changing;
+		pe.pm_credentialschanged = imgp->credential_setid;

filemon already does it this way — filemon_wrapper.c:127-129 pairs the same priv_check_cred(cred, PRIV_DEBUG_DIFFCRED) with imgp->newcred != NULL && imgp->credential_setid.
With this change, my 32 test cases pass (= no regression to what we have right now, except the security issue gone).

Yes, thanks. Probably exec should just be passing imgp directly to these hooks. (HWT has a nearly duplicate hook, but it doesn't seem to check for setuid/setgid switches. OTOH it is currently only usable by privileged users, at least today. Something to fix, but not in this patch.)

My tests detect a regression:

Comments reworded, a part moved into the commit log. The single (after f6afed726b00 ) goto moved into the one place where it matters.

Make the padding explicit.

Move the variables instruct iflib_txq to a place which causes less padding. Make the existing padding in the struct visible. Checked for amd64, riscv64, arm64, and i386 (only arch where the sizeof() changes, follow-up commit to remove the "old" watchdog parts changes the size back.

full context

full context

Results from the affected hardware (em0, 82541PI), ~44 h
on a v3 kernel. Summary: 16 watchdog fires, all true positives, 0 false
positives.

New tested version, on hardware which triggers the watchdog.