Hi,
Checking the fragment flags of an IP packet does not need the mutex
for the fragment list. Move this code before the critical section.
Use ISSET() to make obvious which flags are checked.
ok?
bluhm
Index: netinet/ip_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.374
diff -u -p -U4 -r1.374 ip_input.c
--- netinet/ip_input.c 25 Jul 2022 23:19:34 -0000 1.374
+++ netinet/ip_input.c 27 Jul 2022 22:43:55 -0000
@@ -582,56 +582,57 @@ ip_fragcheck(struct mbuf **mp, int *offp
ip = mtod(*mp, struct ip *);
hlen = ip->ip_hl << 2;
/*
- * If offset or IP_MF are set, must reassemble.
+ * If offset or more fragments are set, must reassemble.
* Otherwise, nothing need be done.
* (We could look in the reassembly queue to see
* if the packet was previously fragmented,
* but it's not worth the time; just let them time out.)
*/
- if (ip->ip_off &~ htons(IP_DF | IP_RF)) {
+ if (ISSET(ip->ip_off, htons(IP_OFFMASK | IP_MF))) {
if ((*mp)->m_flags & M_EXT) { /* XXX */
if ((*mp = m_pullup(*mp, hlen)) == NULL) {
ipstat_inc(ips_toosmall);
return IPPROTO_DONE;
}
ip = mtod(*mp, struct ip *);
}
- mtx_enter(&ipq_mutex);
-
- /*
- * Look for queue of fragments
- * of this datagram.
- */
- LIST_FOREACH(fp, &ipq, ipq_q) {
- if (ip->ip_id == fp->ipq_id &&
- ip->ip_src.s_addr == fp->ipq_src.s_addr &&
- ip->ip_dst.s_addr == fp->ipq_dst.s_addr &&
- ip->ip_p == fp->ipq_p)
- break;
- }
-
/*
* Adjust ip_len to not reflect header,
* set ipqe_mff if more fragments are expected,
* convert offset of this to bytes.
*/
ip->ip_len = htons(ntohs(ip->ip_len) - hlen);
- mff = (ip->ip_off & htons(IP_MF)) != 0;
+ mff = ISSET(ip->ip_off, htons(IP_MF));
if (mff) {
/*
* Make sure that fragments have a data length
* that's a non-zero multiple of 8 bytes.
*/
if (ntohs(ip->ip_len) == 0 ||
(ntohs(ip->ip_len) & 0x7) != 0) {
ipstat_inc(ips_badfrags);
- goto bad;
+ m_freemp(mp);
+ return IPPROTO_DONE;
}
}
ip->ip_off = htons(ntohs(ip->ip_off) << 3);
+
+ mtx_enter(&ipq_mutex);
+
+ /*
+ * Look for queue of fragments
+ * of this datagram.
+ */
+ LIST_FOREACH(fp, &ipq, ipq_q) {
+ if (ip->ip_id == fp->ipq_id &&
+ ip->ip_src.s_addr == fp->ipq_src.s_addr &&
+ ip->ip_dst.s_addr == fp->ipq_dst.s_addr &&
+ ip->ip_p == fp->ipq_p)
+ break;
+ }
/*
* If datagram marked as having more fragments
* or if this is not the first fragment,
