Page MenuHomeFreeBSD
Differential D25486

Improve error path in BBR
ClosedPublic
Actions

Authored by tuexen on Jun 27 2020, 1:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 31, 1:59 PM
Unknown Object (File)
Mar 7 2025, 2:51 PM
Unknown Object (File)
Jan 23 2025, 1:45 PM
Unknown Object (File)
Dec 28 2024, 4:51 PM
Unknown Object (File)
Dec 23 2024, 3:04 PM
Unknown Object (File)
Dec 20 2024, 9:50 PM
Unknown Object (File)
Nov 26 2024, 9:48 AM
Unknown Object (File)
Nov 26 2024, 7:39 AM
View All 40 Files
Subscribers
imp
melifaro

Details

Reviewers
rrs
rscheff
Group Reviewers
transport
Commits
rS362846: Fix the cleanup handling in a error path for TCP BBR.
Summary

syzkaller found a bug in the BBR code. On an error path, the socket buffer was not unlocked. Free also on this path the allocated mbuf.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 32009

Event Timeline

tuexen created this revision.Jun 27 2020, 1:56 PM
Herald added a reviewer: transport. · View Herald TranscriptJun 27 2020, 1:56 PM
Herald added subscribers: melifaro, imp. · View Herald Transcript
tuexen requested review of this revision.Jun 27 2020, 1:56 PM
Harbormaster completed remote builds in B32009: Diff 73760.Jun 27 2020, 1:57 PM
tuexen edited the summary of this revision. (Show Details)Jun 27 2020, 1:57 PM
rscheff accepted this revision.Jun 27 2020, 3:04 PM

This mechanical change looks good to me. Just wondering is similar missing sbunlock would be in RACK or elsewhere in BBR. (but didn't spot anything obvious just now)

This revision is now accepted and ready to land.Jun 27 2020, 3:04 PM
tuexen added a comment.Jun 27 2020, 3:11 PM
In D25486#562860, @rscheff wrote:

This mechanical change looks good to me. Just wondering is similar missing sbunlock would be in RACK or elsewhere in BBR. (but didn't spot anything obvious just now)

syzkaller triggered the condition. I looked for other cases, but did not spot any. That doesn't mean that they don't exist... Maybe syzkaller will find other issue, I don't know. That is the reason why it it constantly running...

Closed by commit rS362846: Fix the cleanup handling in a error path for TCP BBR. (authored by tuexen). · Explain WhyJul 1 2020, 5:17 PM
This revision was automatically updated to reflect the committed changes.
tuexen added a commit: rS362846: Fix the cleanup handling in a error path for TCP BBR..

Revision Contents
Changeset List

PathSize
sys/
netinet/
tcp_stacks/
2 lines

Diff 73760

View Options

sys/netinet/tcp_stacks/bbr.c

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 13,375 Lines • ▼ Show 20 Lines#endif
sack_rxmit = 1; sack_rxmit = 1;
if (rsm->r_start != tp->snd_una) { if (rsm->r_start != tp->snd_una) {
/* /*
* Things are really messed up, <c> * Things are really messed up, <c>
* is the only thing to do. * is the only thing to do.
*/ */
BBR_STAT_INC(bbr_offset_drop); BBR_STAT_INC(bbr_offset_drop);
tcp_set_inp_to_drop(inp, EFAULT); tcp_set_inp_to_drop(inp, EFAULT);
SOCKBUF_UNLOCK(sb);
(void)m_free(m);
return (0); return (0);
} }
len = rsm->r_end - rsm->r_start; len = rsm->r_end - rsm->r_start;
} }
if (len > sbavail(sb)) if (len > sbavail(sb))
len = sbavail(sb); len = sbavail(sb);
if (len > maxseg) if (len > maxseg)
len = maxseg; len = maxseg;
▲ Show 20 Lines • Show All 1,725 Lines • Show Last 20 Lines