Re: ipfilter causing kernel panic in FreeBSD 6.1

Darren Reed wrote:
> Steve Clark wrote:
>
>>Darren Reed wrote:
>>
>>>Steve Clark wrote:
>>>
>>>
>>>>...
>>>>Hi Darren,
>>>>
>>>>Since I knew it was night time "down under" I went ahead and just
>>>>changed to code
>>>>to print a message and return -1 in nat_finalise() then the tcp
>>>>pointer was null.
>>>>The system has stayed up now almost 24 hours - where yesterday it
>>>>had 20 panics.
>>>>
>>>>Is there some reason we wouldn't want to just dump/ignore this
>>>>packet, since it seems to
>>>>me that the initial syn and at least the first packet of the
>>>>fragmented series had gotten lost
>>>>and eventually will be retried.
>>>
>>>
>>>Well, the original packet that caused this problem was not
>>>part of a SYN - it was a midstream TCP packet. I suspect
>>>something else is going on there. It may be part of a retransmitted
>>>packet sent after the original NAT session is torn down.
>>>I don't know. But obviously your policy allows the packet
>>>to be sent (and received) so the NAT code should make
>>>this possible.
>>>
>>>It might be more appropriate to not allow fragments that are
>>>not the first fragment (offset = 0) to form NAT sessions for
>>>the TCP/UDP protocols. That I could understand.
>>>
>>
>>Yes that is what I was getting at. It is my understanding that in
>>freebsd ip_nat happens
>>before ip_filter - so we really can't filter these before they hit
>>ip_nat. Since they are externally
>>generated we would have to put another fw in front of this firewall to
>>keep them out.
>
>
> Ok, then the patch below might be of use to you then.
>
> Darren
>
>
>
> diff -c -r2.195.2.105 ip_nat.c
> *** ip_nat.c 21 Dec 2007 23:03:24 -0000 2.195.2.105
> --- ip_nat.c 8 Feb 2008 02:27:04 -0000
> ***************
> *** 3804,3809 ****
> --- 3804,3813 ----
> * If there is no current entry in the nat table for
> this IP#,
> * create one for it (if there is a matching rule).
> */
> + if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
> + natfailed = -1;
> + goto nonatfrag;
> + }
> RWLOCK_EXIT(&ipf_nat);
> msk = 0xffffffff;
> nmsk = nat_masks;
> ***************
> *** 3861,3866 ****
> --- 3865,3871 ----
> MUTEX_DOWNGRADE(&ipf_nat);
> }
>
> + nonatfrag:
> if (nat != NULL) {
> rval = fr_natout(fin, nat, natadd, nflags);
> if (rval == 1) {
> ***************
> *** 4095,4100 ****
> --- 4100,4109 ----
> } else {
> u_32_t hv, msk, rmsk;
>
> + if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
> + natfailed = -1;
> + goto nonatfrag;
> + }
> RWLOCK_EXIT(&ipf_nat);
> rmsk = rdr_masks;
> msk = 0xffffffff;
> ***************
> *** 4155,4160 ****
> --- 4164,4171 ----
> }
> MUTEX_DOWNGRADE(&ipf_nat);
> }
> +
> + nonatfrag:
> if (nat != NULL) {
> rval = fr_natin(fin, nat, natadd, nflags);
> if (rval == 1) {
>
>
Yes Darren - I like this better.

Thanks,
Steve