Page MenuHomeFreeBSD
Differential D35428

tcp: remove goto and address another NULL deref in SACK
ClosedPublic
Actions

Authored by rscheff on Jun 8 2022, 6:44 AM.
Tags
None
Referenced Files
F166319142: D35428.id106756.diff
Wed, Aug 12, 9:23 PM
F166316022: D35428.id.diff
Wed, Aug 12, 8:52 PM
Unknown Object (File)
Wed, Aug 12, 11:21 AM
Unknown Object (File)
Tue, Aug 11, 10:40 PM
Unknown Object (File)
Tue, Aug 11, 8:29 PM
Unknown Object (File)
Mon, Aug 10, 8:38 AM
Unknown Object (File)
Mon, Aug 10, 8:03 AM
Unknown Object (File)
Mon, Aug 10, 7:52 AM
View All Files
Subscribers
glebius
• hselasky
imp
melifaro
Contributor Reviews (src)

Details

Reviewers
• hselasky
Group Reviewers
transport
Commits
rGce2525c8108a: tcp: remove goto and address another NULL deref in SACK
Summary

Missed another NULL dereference during KASSERTS after traversing
the scoreboard. While at it, scratch the goto by making the
traversal conditional.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 45903
Build 42791: arc lint + arc unit

Event Timeline

rscheff created this revision.Jun 8 2022, 6:44 AM
Herald added a reviewer: transport. · View Herald TranscriptJun 8 2022, 6:44 AM
Herald added subscribers: Contributor Reviews (src), glebius, melifaro, imp. · View Herald Transcript
rscheff requested review of this revision.Jun 8 2022, 6:44 AM
Harbormaster completed remote builds in B45902: Diff 106753.Jun 8 2022, 6:44 AM
• hselasky added a subscriber: • hselasky.Jun 8 2022, 7:03 AM
• hselasky added inline comments.
sys/netinet/tcp_sack.c
971–972

The NULL check should be inside the if (SEQ_GEQ()).

• hselasky added inline comments.Jun 8 2022, 7:05 AM
sys/netinet/tcp_sack.c
962

I would just write the code like this:

for (;;) {
hole = TAILQ_NEXT(hole, scblink);
if (hole == NULL)
return (hole);
if ....
   break;
}
962

To avoid duplicated NULL checks.

rscheff updated this revision to Diff 106754.Jun 8 2022, 7:11 AM
  • unconditional loop, with checks inside
Harbormaster completed remote builds in B45903: Diff 106754.Jun 8 2022, 7:11 AM
rscheff marked 3 inline comments as done.Jun 8 2022, 7:12 AM
• hselasky accepted this revision.Jun 8 2022, 7:15 AM

Looks good!

This revision was not accepted when it landed; it landed in state Needs Review.Jun 8 2022, 7:39 AM
Closed by commit rGce2525c8108a: tcp: remove goto and address another NULL deref in SACK (authored by rscheff). · Explain Why
This revision was automatically updated to reflect the committed changes.
rscheff added a commit: rGce2525c8108a: tcp: remove goto and address another NULL deref in SACK.

Revision Contents
Changeset List

PathSize
sys/
netinet/
16 lines

Diff 106754

View Options

sys/netinet/tcp_sack.c

Loading...