CVS commit: src/sys/dist/pf/net

[David Young]

Module Name: src
Committed By: dyoung
Date: Mon Dec 4 02:58:06 UTC 2006
src/sys/dist/pf/net: pf.c pfvar.h
Lightly constify. Helps compile-time checking that we are not
scribbling over shared or read-only memory---e.g., in mbufs.

Why?

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.

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. A better
fix would use your safe mbuf macros throughout pf. (What is the status
of that, anyway?)

I am afraid that such changes will make merging new versions of pf more
difficult. (Have you cousulted with Peter Postma?)

I did not consult with Peter. I think that this will make merging
negligibly more difficult.

Dave

[Pavel Cahyna]

Post by David Young
I was tracking a bug where pf corrupted packets. Making subroutine
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.

Post by David Young

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.

Post by David Young
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

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[YAMAMOTO Takashi]

Post by David Young

I am afraid that such changes will make merging new versions of pf more
difficult. (Have you cousulted with Peter Postma?)

I did not consult with Peter. I think that this will make merging
negligibly more difficult.

please submit changes to the upstream author.

YAMAMOTO Takashi

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[David Young]

Post by YAMAMOTO Takashi

Post by David Young

I am afraid that such changes will make merging new versions of pf more
difficult. (Have you cousulted with Peter Postma?)

I did not consult with Peter. I think that this will make merging
negligibly more difficult.

please submit changes to the upstream author.

Will do.

Dave