Page MenuHomeFreeBSD

Reapply "Assert that mbufs are writable if we write to them"
Needs ReviewPublic

Authored by jhb on Sep 25 2024, 3:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 3, 7:32 AM
Unknown Object (File)
Sat, Oct 26, 10:44 AM
Unknown Object (File)
Wed, Oct 9, 8:42 AM
Unknown Object (File)
Sep 30 2024, 10:41 AM
Unknown Object (File)
Sep 28 2024, 9:26 PM
Unknown Object (File)
Sep 27 2024, 12:54 AM
Unknown Object (File)
Sep 26 2024, 11:39 AM
Unknown Object (File)
Sep 25 2024, 4:17 PM
Subscribers

Details

Reviewers
gallatin
kp
Summary

Use M_WRITABLE_EXTPG since m_copyback supports writing to M_EXTPG
mbufs.

This reverts commit 299175f2e52e1a5cf495316d53a245bea00dfca7.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 59598
Build 56485: arc lint + arc unit

Event Timeline

jhb requested review of this revision.Sep 25 2024, 3:13 PM

@kp are you ok with this change? I think m_unshare can still be resolved orthogonally from the rest of this series.

I'm okay with this, yes.

The if_ovpn changes are on my todo list, but I expect it'll be a few weeks before I have something reviewable.

Hmm, I still get panics from ktls_test. The problem now is not with M_EXTPG mbufs, but the decryption code might (or might not) have a bug. The panic is due to writing to an M_EXT mbuf that is not marked read-only but has a reference count of 2. The reason it has a reference count of 2 is that for TLS decryption we have to "split" mbufs if a TLS record ends in the middle of a boundary, and in that case you end up with shared references to the backing cluster.

I'm not quite sure how to untangle this assumption that a reference count > 1 implies read-only data. There probably are some cases where that is true, but in the case of KTLS, the two mbufs are referencing different parts of the backing cluster, not truly sharing the data, so it is a false positive. I'm not sure how you could fix the assertion to work properly for these cases.