> I've rolled in all of John's changes (thank you very much for those, it
> is nice to have a new pair of eyes look over it!) from the past week or
> so.

I was looking things over and it's not clear why in SunOS/pfil.c
pfil_precheck you have:

        for (; pfh; pfh = pfh->pfil_next)
                if (pfh->pfil_func) {
                        err = (*pfh->pfil_func)(ip, iphlen, qif, out, qpi, mp);
                        if (err || !*mp)
                                break;
                        /*
                         * fr_pullup may have allocated a new buffer.
                         */
                        ip = qpi->qpi_data;
                }

instead of:

        for (; pfh; pfh = pfh->pfil_next)
                if (pfh->pfil_func) {
                        err = (*pfh->pfil_func)(ip, iphlen, qif, out, qpi, mp);
                        /*
                         * fr_pullup may have allocated a new buffer.
                         */
                        ip = qpi->qpi_data;

                        if (err || !*mp)
                                break;
                }

which I believe is less error prone.  Consider the following:

  1) A packet's byte ordering is change prior to the for loop.

  2) One of the pfil_func functions allocates a new buffer and then
     returns an error.

The pfil-next.tar.gz code will refuse to fix the byte ordering before
returning the error which leaves the contents of the packet useless.
This causes a problem in pfilmodwput and pfilmodrput which pass the
packet to putnext if the error was less than zero.  If pfil_precheck
is going to render the packet useless, then it should explicitly
discard it to avoid unexpected behavior.

This of course raises the question of what should pfil_precheck return
for various types of failures.  Currently the code that allocates a new
mblk chain (in order to fix alignment etc) returns -3 if dupb / msgpullup
fails which is consistent with the old code.  This results in pfilmodwput
and pfilmodrput calling putnext.

  a) Considering this is firewall software shouldn't pfilmodwput
     and pfilmodrput discard the packet if pfil_precheck encounters
     an error?

  b) Should ENOBUFS be returned if dup / msgpullup / pullupmsg fails
     (similar to EMSGSIZE being returned for large IP6 packets)?

-- John
-------------------------------------------------------------------------
|   Feith Systems  |   Voice: 1-215-646-8000  |  Email: [EMAIL PROTECTED]  |
|    John Wehle    |     Fax: 1-215-540-5495  |                         |
-------------------------------------------------------------------------

Reply via email to