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

5585

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

5598

And here?

5764

And here?

5879

And here?

5886

And here?

[markj]

sys/netpfil/pf/pf.c

5598

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

[gallatin]

sys/netpfil/pf/pf.c

5598

And here?

5598

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

5764

Yes.

[markj]

Handle more cases pointed out by Kristof.

[markj]

sys/netpfil/pf/pf.c

5879

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

5879

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

5879

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