On Mon, Dec 04, 2006 at 10:51:28AM -0600, David Young wrote:
> I was tracking a bug where pf corrupted packets.  Making subroutine
> arguments const made it easier to winnow code paths from consideration:
> if a pointer to mbuf storage was passed as const *, I knew it would not
> be overwritten deliberately.

If you suspect deliberate overwriting, one way is to map mbufs read-only.
This way I made sure that the whole network input path was made safe
during my mbuf API work.

> > mbufs passed to pf are guaranteed to be writable. (see PR 26433)
> 
> I did not think that the fix in 26433 was intended as anything but
> a stopgap.  It does not seem efficient to copy IP+(UDP|TCP) headers on
> every single packet regardless of whether it will be modified.

True, it is inefficent, but it should work correctly. Do you know how your
corruption happened? I don't see how could overwriting r/o packets be a
problem, given that with the "stopgap fix" pf should never see r/o
packets.

> A better
> fix would use your safe mbuf macros throughout pf.  (What is the status
> of that, anyway?)

It would be a better fix, but pf is not under our control. I don't think
that making extensive local changes is reasonable, but I am not the person
who will do future updates of pf...

Status is that I've fixed one bug that I knew about and changed the
API to be more consistent. Now my plan is to merge the new API Really
Soon, and then do the conversion of existing code to it in separate
commits as time permits. (Also depending on the amount of conflicting
changes that happened since I imported the code in May.)

Pavel