Re: ip_reass() - possibly incorrect goto

2005-04-08 Thread Maxim Konovalov
On Thu, 24 Mar 2005, 04:46-0600, Mike Silbersack wrote:


 On Wed, 23 Mar 2005, Maxim Konovalov wrote:

  On Tue, 22 Mar 2005, 12:08-0800, [EMAIL PROTECTED] wrote:
 
   Hi hackers, I am looking at the ip_reass() routine. In case of the
   1st fragment we create the reassembly queue. After the queue has
   been inserted in the hash bucket, the if () code does a  goto
   inserted. Should this be changed to goto done instead? Any code
   that is executed for the 1st fragment, like frag per packet limiting
   and complete reassembly are not valid. Am I mistaken?
 
  Yep, it seems you are right.  The second micro optimization - drop the
  fragment early if maxfragsperpacket == 0.
 
  Andre, Mike, what do you think?

 Looks good to me.  Please tell us if you come up with any more optimizations
 for the reassembly code, Vijay.
[...]

Committed to HEAD.  Thanks, Vijay!

-- 
Maxim Konovalov
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: ip_reass() - possibly incorrect goto

2005-03-24 Thread Mike Silbersack
On Wed, 23 Mar 2005, Maxim Konovalov wrote:
On Tue, 22 Mar 2005, 12:08-0800, [EMAIL PROTECTED] wrote:
Hi hackers, I am looking at the ip_reass() routine. In case of the
1st fragment we create the reassembly queue. After the queue has
been inserted in the hash bucket, the if () code does a  goto
inserted. Should this be changed to goto done instead? Any code
that is executed for the 1st fragment, like frag per packet limiting
and complete reassembly are not valid. Am I mistaken?
Yep, it seems you are right.  The second micro optimization - drop the
fragment early if maxfragsperpacket == 0.
Andre, Mike, what do you think?
Looks good to me.  Please tell us if you come up with any more 
optimizations for the reassembly code, Vijay.

On a related note...
While looking through the code, I think I figured out a way to avoid IDSes 
if you're trying to mess with a FreeBSD machine:

/*
 * Handle ECN by comparing this segment with the first one;
 * if CE is set, do not lose CE.
 * drop if CE and not-ECT are mixed for the same packet.
 */
Couldn't you send a fragment with half the exploit payload (too short 
for the IDS to match), then send a packet with a different ECN status to 
overwrite that fragment (at least in the IDS's buffer, but not in 
FreeBSD's, since it would be dropped), and then send the second part of 
the payload?

Hmmm...
Mike Silby Silbersack
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


Re: ip_reass() - possibly incorrect goto

2005-03-23 Thread Maxim Konovalov
On Tue, 22 Mar 2005, 12:08-0800, [EMAIL PROTECTED] wrote:

 Hi hackers, I am looking at the ip_reass() routine. In case of the
 1st fragment we create the reassembly queue. After the queue has
 been inserted in the hash bucket, the if () code does a  goto
 inserted. Should this be changed to goto done instead? Any code
 that is executed for the 1st fragment, like frag per packet limiting
 and complete reassembly are not valid. Am I mistaken?

Yep, it seems you are right.  The second micro optimization - drop the
fragment early if maxfragsperpacket == 0.

Andre, Mike, what do you think?

Index: ip_input.c
===
RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.299
diff -u -r1.299 ip_input.c
--- ip_input.c  16 Mar 2005 05:27:19 -  1.299
+++ ip_input.c  23 Mar 2005 13:12:00 -
@@ -801,8 +801,8 @@
u_int8_t ecn, ecn0;
u_short hash;

-   /* If maxnipq is 0, never accept fragments. */
-   if (maxnipq == 0) {
+   /* If maxnipq or maxfragsperpacket are 0, never accept fragments. */
+   if (maxnipq == 0 || maxfragsperpacket == 0) {
ipstat.ips_fragments++;
ipstat.ips_fragdropped++;
m_freem(m);
@@ -918,7 +918,7 @@
fp-ipq_dst = ip-ip_dst;
fp-ipq_frags = m;
m-m_nextpkt = NULL;
-   goto inserted;
+   goto done;
} else {
fp-ipq_nfrags++;
 #ifdef MAC
@@ -998,8 +998,6 @@
m_freem(q);
}

-inserted:
-
/*
 * Check for complete reassembly and perform frag per packet
 * limiting.
%%%

-- 
Maxim Konovalov
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]


ip_reass() - possibly incorrect goto

2005-03-22 Thread Vijay.Singh
Hi hackers, I am looking at the ip_reass() routine. In case of the 1st fragment 
we create the reassembly queue. After the queue has been inserted in the hash 
bucket, the if () code does a  goto inserted. Should this be changed to goto 
done instead? Any code that is executed for the 1st fragment, like frag per 
packet limiting and complete reassembly are not valid. Am I mistaken?

br
vijay
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]