pf: Handle unmapped mbufs when computing checksums

[markj]

Summary

PR: 254419

[markj]

I see a call in ipfw, I guess that needs to be updated too. Seems time to introduce a new subroutine to handle this.

[kp]

sys/netpfil/pf/pf.c

5582

Presumably we'd have to do this here as well.

5595

And here?

5758

And here?

5873

And here?

5880

And here?

[markj]

sys/netpfil/pf/pf.c

5595

I believe this treatment isn't needed when we're only checksumming a protocol header.

[gallatin]

sys/netpfil/pf/pf.c

5595

And here?

5595

No, since this is just the IP header. Unmapped mbufs are a problem for checksums only on payload data.

5758

Yes.

[markj]

Handle more cases pointed out by Kristof.

[markj]

sys/netpfil/pf/pf.c

5873

These seem a bit tricky. mb_unmapped_to_ext() frees the chain upon failure, but this function and its callers can't really do that. We are also assuming in general that the first mbuf in the chain is preserved, otherwise mb_unmapped_to_ext() would invalidate some pointers in its callers.

[kp]

sys/netpfil/pf/pf.c

5873

It's tempting to 'fix' this by calling mb_unmapped_to_ext() in pf_check_in()/pf_check_out(), so that we restore the assumption that it's safe to access packet data.
Probably not the best idea though.

Is there a straightforward way of asserting that an mbuf is or is not unmapped? Just adding an assertion here would at least make future debugging easier.

[jhb]

sys/netpfil/pf/pf.c

5873

You have to check the entire chain to see if any mbufs in the chain have M_EXTPG set in m_flags. However, it might be nice to have some kind of M_ASSERTMAPPED(m) wrapper for that the way we have a M_ASSERTPKTHDR() macro.

[ae]

In D29378#657837, @markj wrote:

I see a call in ipfw, I guess that needs to be updated too. Seems time to introduce a new subroutine to handle this.

It seems we have first report about panic in the related code: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=255164