Page MenuHomeFreeBSD
Differential D43051

Make kldxref a bootstrap tool and use unconditionally
ClosedPublic
Actions

Authored by jrtc27 on Dec 13 2023, 8:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 31, 10:08 AM
Unknown Object (File)
Sun, Mar 9, 5:21 PM
Unknown Object (File)
Mar 2 2025, 1:25 PM
Unknown Object (File)
Feb 25 2025, 11:37 AM
Unknown Object (File)
Feb 3 2025, 12:15 PM
Unknown Object (File)
Jan 28 2025, 3:09 AM
Unknown Object (File)
Jan 21 2025, 2:42 PM
Unknown Object (File)
Jan 17 2025, 6:19 PM
View All 55 Files
Subscribers
None

Details

Reviewers
brooks
emaste
imp
jhb
Commits
rG2fbed5e9ea84: Make kldxref a bootstrap tool and use unconditionally
rGff7c12c1f17e: Make kldxref a bootstrap tool and use unconditionally
Summary

Now that kldxref is a generic cross tool and can be built on non-FreeBSD
we can bootstrap it during the build and thus remove the condition for
whether it exists. We also need to make sure to add it to the METALOG
for -DNO_ROOT builds.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jrtc27 requested review of this revision.Dec 13 2023, 8:27 PM
jrtc27 created this revision.
Harbormaster completed remote builds in B54933: Diff 131390.Dec 13 2023, 8:27 PM
jrtc27 added parent revisions: D43049: tools/build: Provide sys/linker_set.h when cross-building, D43050: Makefile.inc1: Forward on METALOG and DISTBASE for kernel targets.Dec 13 2023, 8:28 PM
imp accepted this revision.Dec 13 2023, 8:29 PM
This revision is now accepted and ready to land.Dec 13 2023, 8:29 PM
brooks accepted this revision.Dec 13 2023, 9:34 PM
brooks added inline comments.
sys/conf/kmod.mk
365

Should this be indented 4-spaces instead of a tab as a shell continuation?

jrtc27 added inline comments.Dec 13 2023, 9:36 PM
sys/conf/kmod.mk
365

Yeah, it should, I copied it from somewhere that got it wrong. Will fix when committing.

Closed by commit rGff7c12c1f17e: Make kldxref a bootstrap tool and use unconditionally (authored by jrtc27). · Explain WhyDec 13 2023, 9:44 PM
This revision was automatically updated to reflect the committed changes.
jrtc27 added a commit: rGff7c12c1f17e: Make kldxref a bootstrap tool and use unconditionally.
jhb added a commit: rG2fbed5e9ea84: Make kldxref a bootstrap tool and use unconditionally.Jan 18 2024, 10:32 PM

Revision Contents
Changeset List

PathSize
5 lines
sys/
conf/
9 lines
modules/
9 lines
tools/
build/
2 lines

Diff 131399

View Options

Makefile.inc1

Show First 20 Lines • Show All 196 Lines • ▼ Show 20 LinesSYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_syn, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_syn), 0, &VNET_NAME(tcp_insecure_syn), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting SYN packets"); "Follow RFC793 instead of RFC5961 criteria for accepting SYN packets");
VNET_DEFINE(int, tcp_insecure_rst) = 0; VNET_DEFINE(int, tcp_insecure_rst) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_rst), 0, &VNET_NAME(tcp_insecure_rst), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting RST packets"); "Follow RFC793 instead of RFC5961 criteria for accepting RST packets");
VNET_DEFINE(int, tcp_insecure_ack) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_ack, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_ack), 0,
"Follow RFC793 criteria for validating SEG.ACK");
VNET_DEFINE(int, tcp_recvspace) = 1024*64; VNET_DEFINE(int, tcp_recvspace) = 1024*64;
#define V_tcp_recvspace VNET(tcp_recvspace) #define V_tcp_recvspace VNET(tcp_recvspace)
SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size"); &VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size");
VNET_DEFINE(int, tcp_do_autorcvbuf) = 1; VNET_DEFINE(int, tcp_do_autorcvbuf) = 1;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_do_autorcvbuf), 0, &VNET_NAME(tcp_do_autorcvbuf), 0,
▲ Show 20 Lines • Show All 2,018 Lines • ▼ Show 20 Lines if (V_tcp_insecure_syn &&
/* Send challenge ACK. */ /* Send challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt, tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt,
tp->snd_nxt, TH_ACK); tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt; tp->last_ack_sent = tp->rcv_nxt;
m = NULL; m = NULL;
} }
goto drop; goto drop;
} }
ccUnsubmitted
Done
Inline Actions

At the server side (listening side), I am thinking if we move the secure ACK check here and above the TOF_TS check, it would save cycles on checking these timestamps.

cc: At the server side (listening side), I am thinking if we move the secure ACK check here and…
tuexenAuthorUnsubmitted
Done
Inline Actions

This patch is only about improving the input validation, not about changing the sequence of checks.
Please note that RFC 7323 requires to perform the timestamp checks before the SEG.ACK number checks.

tuexen: This patch is only about improving the input validation, not about changing the sequence of…
ccUnsubmitted
Done
Inline Actions

I see, in the section 5.3. Basic PAWS Algorithm from RFC7323 claims Also, PAWS processing MUST take precedence over the regular TCP acceptability check (Section 3.3 in [RFC0793]), which is performed after verification of the received Timestamps option...

cc: I see, in the section `5.3. Basic PAWS Algorithm` from RFC7323 claims `Also, PAWS processing…
/* /*
* RFC 1323 PAWS: If we have a timestamp reply on this segment * RFC 1323 PAWS: If we have a timestamp reply on this segment
* and it's less than ts_recent, drop it. * and it's less than ts_recent, drop it.
*/ */
if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent && if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent &&
TSTMP_LT(to.to_tsval, tp->ts_recent)) { TSTMP_LT(to.to_tsval, tp->ts_recent)) {
/* Check to see if ts_recent is over 24 days old. */ /* Check to see if ts_recent is over 24 days old. */
if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) { if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) {
▲ Show 20 Lines • Show All 185 Lines • ▼ Show 20 Lines if (tp->t_state == TCPS_SYN_RECEIVED ||
goto dropafterack; goto dropafterack;
else else
goto drop; goto drop;
} }
/* /*
* Ack processing. * Ack processing.
*/ */
if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) {
ccUnsubmitted
Done
Inline Actions

Can use TCP_MAXWIN instead of 65535.

cc: Can use `TCP_MAXWIN` instead of `65535`.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed in 37b3e6a660f551ceda412e2115748f3b97b2e615, since this review has already been committed.

tuexen: Fixed in [[ https://cgit.FreeBSD.org/src/commit/?id=37b3e6a660f551ceda412e2115748f3b97b2e615 |…
/* Checking SEG.ACK against ISS is definitely redundant. */
tp->t_flags2 |= TF2_NO_ISS_CHECK;
ccUnsubmitted
Done
Inline Actions

code style: As there are two lines, recommend using a pair of {} for readability.

cc: code style: As there are two lines, recommend using a pair of `{}` for readability.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed.

tuexen: Fixed.
}
if (!V_tcp_insecure_ack) {
tcp_seq seq_min;
bool ghost_ack_check;
if (tp->t_flags2 & TF2_NO_ISS_CHECK) {
/* Check for too old ACKs (RFC 5961, Section 5.2). */
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
} else {
if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) {
/* Checking for ghost ACKs is stricter. */
seq_min = tp->iss + 1;
ghost_ack_check = true;
} else {
/*
* Checking for too old ACKs (RFC 5961,
* Section 5.2) is stricter.
*/
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
}
}
if (SEQ_LT(th->th_ack, seq_min)) {
if (ghost_ack_check)
TCPSTAT_INC(tcps_rcvghostack);
else
TCPSTAT_INC(tcps_rcvacktooold);
/* Send a challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m,
tp->rcv_nxt, tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt;
m = NULL;
goto drop;
}
}
switch (tp->t_state) { switch (tp->t_state) {
/* /*
* In SYN_RECEIVED state, the ack ACKs our SYN, so enter * In SYN_RECEIVED state, the ack ACKs our SYN, so enter
* ESTABLISHED state and continue processing. * ESTABLISHED state and continue processing.
* The ACK was checked above. * The ACK was checked above.
*/ */
case TCPS_SYN_RECEIVED: case TCPS_SYN_RECEIVED:
▲ Show 20 Lines • Show All 1,746 Lines • Show Last 20 Lines
View Options

sys/conf/kmod.mk

Show First 20 Lines • Show All 196 Lines • ▼ Show 20 LinesSYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_syn, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_syn), 0, &VNET_NAME(tcp_insecure_syn), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting SYN packets"); "Follow RFC793 instead of RFC5961 criteria for accepting SYN packets");
VNET_DEFINE(int, tcp_insecure_rst) = 0; VNET_DEFINE(int, tcp_insecure_rst) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_rst), 0, &VNET_NAME(tcp_insecure_rst), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting RST packets"); "Follow RFC793 instead of RFC5961 criteria for accepting RST packets");
VNET_DEFINE(int, tcp_insecure_ack) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_ack, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_ack), 0,
"Follow RFC793 criteria for validating SEG.ACK");
VNET_DEFINE(int, tcp_recvspace) = 1024*64; VNET_DEFINE(int, tcp_recvspace) = 1024*64;
#define V_tcp_recvspace VNET(tcp_recvspace) #define V_tcp_recvspace VNET(tcp_recvspace)
SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size"); &VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size");
VNET_DEFINE(int, tcp_do_autorcvbuf) = 1; VNET_DEFINE(int, tcp_do_autorcvbuf) = 1;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_do_autorcvbuf), 0, &VNET_NAME(tcp_do_autorcvbuf), 0,
▲ Show 20 Lines • Show All 2,018 Lines • ▼ Show 20 Lines if (V_tcp_insecure_syn &&
/* Send challenge ACK. */ /* Send challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt, tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt,
tp->snd_nxt, TH_ACK); tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt; tp->last_ack_sent = tp->rcv_nxt;
m = NULL; m = NULL;
} }
goto drop; goto drop;
} }
ccUnsubmitted
Done
Inline Actions

At the server side (listening side), I am thinking if we move the secure ACK check here and above the TOF_TS check, it would save cycles on checking these timestamps.

cc: At the server side (listening side), I am thinking if we move the secure ACK check here and…
tuexenAuthorUnsubmitted
Done
Inline Actions

This patch is only about improving the input validation, not about changing the sequence of checks.
Please note that RFC 7323 requires to perform the timestamp checks before the SEG.ACK number checks.

tuexen: This patch is only about improving the input validation, not about changing the sequence of…
ccUnsubmitted
Done
Inline Actions

I see, in the section 5.3. Basic PAWS Algorithm from RFC7323 claims Also, PAWS processing MUST take precedence over the regular TCP acceptability check (Section 3.3 in [RFC0793]), which is performed after verification of the received Timestamps option...

cc: I see, in the section `5.3. Basic PAWS Algorithm` from RFC7323 claims `Also, PAWS processing…
/* /*
* RFC 1323 PAWS: If we have a timestamp reply on this segment * RFC 1323 PAWS: If we have a timestamp reply on this segment
* and it's less than ts_recent, drop it. * and it's less than ts_recent, drop it.
*/ */
if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent && if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent &&
TSTMP_LT(to.to_tsval, tp->ts_recent)) { TSTMP_LT(to.to_tsval, tp->ts_recent)) {
/* Check to see if ts_recent is over 24 days old. */ /* Check to see if ts_recent is over 24 days old. */
if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) { if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) {
▲ Show 20 Lines • Show All 185 Lines • ▼ Show 20 Lines if (tp->t_state == TCPS_SYN_RECEIVED ||
goto dropafterack; goto dropafterack;
else else
goto drop; goto drop;
} }
/* /*
* Ack processing. * Ack processing.
*/ */
if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) {
ccUnsubmitted
Done
Inline Actions

Can use TCP_MAXWIN instead of 65535.

cc: Can use `TCP_MAXWIN` instead of `65535`.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed in 37b3e6a660f551ceda412e2115748f3b97b2e615, since this review has already been committed.

tuexen: Fixed in [[ https://cgit.FreeBSD.org/src/commit/?id=37b3e6a660f551ceda412e2115748f3b97b2e615 |…
/* Checking SEG.ACK against ISS is definitely redundant. */
tp->t_flags2 |= TF2_NO_ISS_CHECK;
ccUnsubmitted
Done
Inline Actions

code style: As there are two lines, recommend using a pair of {} for readability.

cc: code style: As there are two lines, recommend using a pair of `{}` for readability.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed.

tuexen: Fixed.
}
if (!V_tcp_insecure_ack) {
tcp_seq seq_min;
bool ghost_ack_check;
if (tp->t_flags2 & TF2_NO_ISS_CHECK) {
/* Check for too old ACKs (RFC 5961, Section 5.2). */
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
} else {
if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) {
/* Checking for ghost ACKs is stricter. */
seq_min = tp->iss + 1;
ghost_ack_check = true;
} else {
/*
* Checking for too old ACKs (RFC 5961,
* Section 5.2) is stricter.
*/
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
}
}
if (SEQ_LT(th->th_ack, seq_min)) {
if (ghost_ack_check)
TCPSTAT_INC(tcps_rcvghostack);
else
TCPSTAT_INC(tcps_rcvacktooold);
/* Send a challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m,
tp->rcv_nxt, tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt;
m = NULL;
goto drop;
}
}
switch (tp->t_state) { switch (tp->t_state) {
/* /*
* In SYN_RECEIVED state, the ack ACKs our SYN, so enter * In SYN_RECEIVED state, the ack ACKs our SYN, so enter
* ESTABLISHED state and continue processing. * ESTABLISHED state and continue processing.
* The ACK was checked above. * The ACK was checked above.
*/ */
case TCPS_SYN_RECEIVED: case TCPS_SYN_RECEIVED:
▲ Show 20 Lines • Show All 1,746 Lines • Show Last 20 Lines
View Options

sys/modules/Makefile

Show First 20 Lines • Show All 196 Lines • ▼ Show 20 LinesSYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_syn, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_syn), 0, &VNET_NAME(tcp_insecure_syn), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting SYN packets"); "Follow RFC793 instead of RFC5961 criteria for accepting SYN packets");
VNET_DEFINE(int, tcp_insecure_rst) = 0; VNET_DEFINE(int, tcp_insecure_rst) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_rst), 0, &VNET_NAME(tcp_insecure_rst), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting RST packets"); "Follow RFC793 instead of RFC5961 criteria for accepting RST packets");
VNET_DEFINE(int, tcp_insecure_ack) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_ack, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_ack), 0,
"Follow RFC793 criteria for validating SEG.ACK");
VNET_DEFINE(int, tcp_recvspace) = 1024*64; VNET_DEFINE(int, tcp_recvspace) = 1024*64;
#define V_tcp_recvspace VNET(tcp_recvspace) #define V_tcp_recvspace VNET(tcp_recvspace)
SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size"); &VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size");
VNET_DEFINE(int, tcp_do_autorcvbuf) = 1; VNET_DEFINE(int, tcp_do_autorcvbuf) = 1;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_do_autorcvbuf), 0, &VNET_NAME(tcp_do_autorcvbuf), 0,
▲ Show 20 Lines • Show All 2,018 Lines • ▼ Show 20 Lines if (V_tcp_insecure_syn &&
/* Send challenge ACK. */ /* Send challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt, tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt,
tp->snd_nxt, TH_ACK); tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt; tp->last_ack_sent = tp->rcv_nxt;
m = NULL; m = NULL;
} }
goto drop; goto drop;
} }
ccUnsubmitted
Done
Inline Actions

At the server side (listening side), I am thinking if we move the secure ACK check here and above the TOF_TS check, it would save cycles on checking these timestamps.

cc: At the server side (listening side), I am thinking if we move the secure ACK check here and…
tuexenAuthorUnsubmitted
Done
Inline Actions

This patch is only about improving the input validation, not about changing the sequence of checks.
Please note that RFC 7323 requires to perform the timestamp checks before the SEG.ACK number checks.

tuexen: This patch is only about improving the input validation, not about changing the sequence of…
ccUnsubmitted
Done
Inline Actions

I see, in the section 5.3. Basic PAWS Algorithm from RFC7323 claims Also, PAWS processing MUST take precedence over the regular TCP acceptability check (Section 3.3 in [RFC0793]), which is performed after verification of the received Timestamps option...

cc: I see, in the section `5.3. Basic PAWS Algorithm` from RFC7323 claims `Also, PAWS processing…
/* /*
* RFC 1323 PAWS: If we have a timestamp reply on this segment * RFC 1323 PAWS: If we have a timestamp reply on this segment
* and it's less than ts_recent, drop it. * and it's less than ts_recent, drop it.
*/ */
if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent && if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent &&
TSTMP_LT(to.to_tsval, tp->ts_recent)) { TSTMP_LT(to.to_tsval, tp->ts_recent)) {
/* Check to see if ts_recent is over 24 days old. */ /* Check to see if ts_recent is over 24 days old. */
if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) { if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) {
▲ Show 20 Lines • Show All 185 Lines • ▼ Show 20 Lines if (tp->t_state == TCPS_SYN_RECEIVED ||
goto dropafterack; goto dropafterack;
else else
goto drop; goto drop;
} }
/* /*
* Ack processing. * Ack processing.
*/ */
if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) {
ccUnsubmitted
Done
Inline Actions

Can use TCP_MAXWIN instead of 65535.

cc: Can use `TCP_MAXWIN` instead of `65535`.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed in 37b3e6a660f551ceda412e2115748f3b97b2e615, since this review has already been committed.

tuexen: Fixed in [[ https://cgit.FreeBSD.org/src/commit/?id=37b3e6a660f551ceda412e2115748f3b97b2e615 |…
/* Checking SEG.ACK against ISS is definitely redundant. */
tp->t_flags2 |= TF2_NO_ISS_CHECK;
ccUnsubmitted
Done
Inline Actions

code style: As there are two lines, recommend using a pair of {} for readability.

cc: code style: As there are two lines, recommend using a pair of `{}` for readability.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed.

tuexen: Fixed.
}
if (!V_tcp_insecure_ack) {
tcp_seq seq_min;
bool ghost_ack_check;
if (tp->t_flags2 & TF2_NO_ISS_CHECK) {
/* Check for too old ACKs (RFC 5961, Section 5.2). */
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
} else {
if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) {
/* Checking for ghost ACKs is stricter. */
seq_min = tp->iss + 1;
ghost_ack_check = true;
} else {
/*
* Checking for too old ACKs (RFC 5961,
* Section 5.2) is stricter.
*/
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
}
}
if (SEQ_LT(th->th_ack, seq_min)) {
if (ghost_ack_check)
TCPSTAT_INC(tcps_rcvghostack);
else
TCPSTAT_INC(tcps_rcvacktooold);
/* Send a challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m,
tp->rcv_nxt, tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt;
m = NULL;
goto drop;
}
}
switch (tp->t_state) { switch (tp->t_state) {
/* /*
* In SYN_RECEIVED state, the ack ACKs our SYN, so enter * In SYN_RECEIVED state, the ack ACKs our SYN, so enter
* ESTABLISHED state and continue processing. * ESTABLISHED state and continue processing.
* The ACK was checked above. * The ACK was checked above.
*/ */
case TCPS_SYN_RECEIVED: case TCPS_SYN_RECEIVED:
▲ Show 20 Lines • Show All 1,746 Lines • Show Last 20 Lines
View Options

tools/build/Makefile

Show First 20 Lines • Show All 196 Lines • ▼ Show 20 LinesSYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_syn, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_syn), 0, &VNET_NAME(tcp_insecure_syn), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting SYN packets"); "Follow RFC793 instead of RFC5961 criteria for accepting SYN packets");
VNET_DEFINE(int, tcp_insecure_rst) = 0; VNET_DEFINE(int, tcp_insecure_rst) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_rst), 0, &VNET_NAME(tcp_insecure_rst), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting RST packets"); "Follow RFC793 instead of RFC5961 criteria for accepting RST packets");
VNET_DEFINE(int, tcp_insecure_ack) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_ack, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_ack), 0,
"Follow RFC793 criteria for validating SEG.ACK");
VNET_DEFINE(int, tcp_recvspace) = 1024*64; VNET_DEFINE(int, tcp_recvspace) = 1024*64;
#define V_tcp_recvspace VNET(tcp_recvspace) #define V_tcp_recvspace VNET(tcp_recvspace)
SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size"); &VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size");
VNET_DEFINE(int, tcp_do_autorcvbuf) = 1; VNET_DEFINE(int, tcp_do_autorcvbuf) = 1;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_do_autorcvbuf), 0, &VNET_NAME(tcp_do_autorcvbuf), 0,
▲ Show 20 Lines • Show All 2,018 Lines • ▼ Show 20 Lines if (V_tcp_insecure_syn &&
/* Send challenge ACK. */ /* Send challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt, tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt,
tp->snd_nxt, TH_ACK); tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt; tp->last_ack_sent = tp->rcv_nxt;
m = NULL; m = NULL;
} }
goto drop; goto drop;
} }
ccUnsubmitted
Done
Inline Actions

At the server side (listening side), I am thinking if we move the secure ACK check here and above the TOF_TS check, it would save cycles on checking these timestamps.

cc: At the server side (listening side), I am thinking if we move the secure ACK check here and…
tuexenAuthorUnsubmitted
Done
Inline Actions

This patch is only about improving the input validation, not about changing the sequence of checks.
Please note that RFC 7323 requires to perform the timestamp checks before the SEG.ACK number checks.

tuexen: This patch is only about improving the input validation, not about changing the sequence of…
ccUnsubmitted
Done
Inline Actions

I see, in the section 5.3. Basic PAWS Algorithm from RFC7323 claims Also, PAWS processing MUST take precedence over the regular TCP acceptability check (Section 3.3 in [RFC0793]), which is performed after verification of the received Timestamps option...

cc: I see, in the section `5.3. Basic PAWS Algorithm` from RFC7323 claims `Also, PAWS processing…
/* /*
* RFC 1323 PAWS: If we have a timestamp reply on this segment * RFC 1323 PAWS: If we have a timestamp reply on this segment
* and it's less than ts_recent, drop it. * and it's less than ts_recent, drop it.
*/ */
if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent && if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent &&
TSTMP_LT(to.to_tsval, tp->ts_recent)) { TSTMP_LT(to.to_tsval, tp->ts_recent)) {
/* Check to see if ts_recent is over 24 days old. */ /* Check to see if ts_recent is over 24 days old. */
if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) { if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) {
▲ Show 20 Lines • Show All 185 Lines • ▼ Show 20 Lines if (tp->t_state == TCPS_SYN_RECEIVED ||
goto dropafterack; goto dropafterack;
else else
goto drop; goto drop;
} }
/* /*
* Ack processing. * Ack processing.
*/ */
if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) {
ccUnsubmitted
Done
Inline Actions

Can use TCP_MAXWIN instead of 65535.

cc: Can use `TCP_MAXWIN` instead of `65535`.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed in 37b3e6a660f551ceda412e2115748f3b97b2e615, since this review has already been committed.

tuexen: Fixed in [[ https://cgit.FreeBSD.org/src/commit/?id=37b3e6a660f551ceda412e2115748f3b97b2e615 |…
/* Checking SEG.ACK against ISS is definitely redundant. */
tp->t_flags2 |= TF2_NO_ISS_CHECK;
ccUnsubmitted
Done
Inline Actions

code style: As there are two lines, recommend using a pair of {} for readability.

cc: code style: As there are two lines, recommend using a pair of `{}` for readability.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed.

tuexen: Fixed.
}
if (!V_tcp_insecure_ack) {
tcp_seq seq_min;
bool ghost_ack_check;
if (tp->t_flags2 & TF2_NO_ISS_CHECK) {
/* Check for too old ACKs (RFC 5961, Section 5.2). */
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
} else {
if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) {
/* Checking for ghost ACKs is stricter. */
seq_min = tp->iss + 1;
ghost_ack_check = true;
} else {
/*
* Checking for too old ACKs (RFC 5961,
* Section 5.2) is stricter.
*/
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
}
}
if (SEQ_LT(th->th_ack, seq_min)) {
if (ghost_ack_check)
TCPSTAT_INC(tcps_rcvghostack);
else
TCPSTAT_INC(tcps_rcvacktooold);
/* Send a challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m,
tp->rcv_nxt, tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt;
m = NULL;
goto drop;
}
}
switch (tp->t_state) { switch (tp->t_state) {
/* /*
* In SYN_RECEIVED state, the ack ACKs our SYN, so enter * In SYN_RECEIVED state, the ack ACKs our SYN, so enter
* ESTABLISHED state and continue processing. * ESTABLISHED state and continue processing.
* The ACK was checked above. * The ACK was checked above.
*/ */
case TCPS_SYN_RECEIVED: case TCPS_SYN_RECEIVED:
▲ Show 20 Lines • Show All 1,746 Lines • Show Last 20 Lines