Page MenuHomeFreeBSD
Differential D42946

tcp_hpts: let tcp_hpts_init() set a random CPU only once
ClosedPublic
Actions

Authored by glebius on Dec 7 2023, 1:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Feb 27, 5:44 AM
Unknown Object (File)
Wed, Feb 26, 9:09 AM
Unknown Object (File)
Tue, Feb 25, 11:49 PM
Unknown Object (File)
Tue, Feb 25, 8:11 PM
Unknown Object (File)
Sat, Feb 22, 2:21 PM
Unknown Object (File)
Sat, Feb 22, 2:00 PM
Unknown Object (File)
Sat, Feb 22, 1:42 PM
Unknown Object (File)
Thu, Feb 20, 11:08 AM
View All 52 Files
Subscribers
imp
melifaro

Details

Reviewers
rrs
tuexen
Group Reviewers
transport
Commits
rGc0303fa84eed: tcp_hpts: let tcp_hpts_init() set a random CPU only once
rG3f46be6acadd: tcp_hpts: let tcp_hpts_init() set a random CPU only once
Summary

After d2ef52ef3dee the tcp_hpts_init() function can be called multiple
times on a tcpcb if it is switched there and back between two TCP stacks.
First, this makes existing assertion in tcp_hpts_init() incorrect. Second,
it creates possibility to change a randomly set t_hpts_cpu to a different
random value, while a tcpcb is already in the HPTS wheel, triggering other
assertions later in tcp_hptsi().

The best approach here would be to work on the stacks to really clear a
tcpcb out of HPTS wheel in tfb_tcp_fb_fini, draining the IHPTS_MOVING
state. But that's pretty intrusive change, so let's just get back to the
old logic (pre d2ef52ef3dee) where t_hpts_cpu was set to a random value
only once in a CPU lifetime and a newly switched stack inherits t_hpts_cpu
from the previous stack.

Reported-by: syzbot+fab29fe1ab089c52998d@syzkaller.appspotmail.com
Reported-by: syzbot+ca5f2aa0fda15dcfe6d7@syzkaller.appspotmail.com
Fixes: 2b3a77467dd3d74a7170f279fb25f9736b46ef8a

Diff Detail

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

Revision Contents
Changeset List

PathSize
sys/
netinet/
16 lines
3 lines

Diff 131154

View Options

sys/netinet/tcp_hpts.c

Show All 13 Lines
.\" THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR .\" THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
.\" IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, .\" IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
.\" FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE .\" FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
.\" AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER .\" AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
.\" LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, .\" LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
.\" OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN .\" OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
.\" THE SOFTWARE. .\" THE SOFTWARE.
.\" .\"
.Dd May 22, 2024 .Dd August 4, 2024
.Dt MIXER 3 .Dt MIXER 3
.Os .Os
.Sh NAME .Sh NAME
.Nm mixer_open , .Nm mixer_open ,
.Nm mixer_close , .Nm mixer_close ,
.Nm mixer_get_dev , .Nm mixer_get_dev ,
.Nm mixer_get_dev_byname , .Nm mixer_get_dev_byname ,
.Nm mixer_add_ctl , .Nm mixer_add_ctl ,
▲ Show 20 Lines • Show All 352 Lines • ▼ Show 20 Lines
functions get and set the default audio card in the system. functions get and set the default audio card in the system.
Although this is not really a mixer feature, it is useful to have instead of \ Although this is not really a mixer feature, it is useful to have instead of \
having to use the having to use the
.Xr sysctl 3 .Xr sysctl 3
controls. controls.
.Pp .Pp
The The
.Fn mixer_get_mode .Fn mixer_get_mode
function returns the playback/recording mode of the audio device the mixer \ function returns the operating mode of the audio device the mixer belongs to.
belongs to. The following values can be OR'ed in case more than one mode is supported:
dev_submerge.chUnsubmitted
Done
Inline Actions

"They"?

dev_submerge.ch: "They"?
christosAuthorUnsubmitted
Done
Inline Actions

Typo.

christos: Typo.
The available values are the following: .Bl -tag -width "MIX_MODE_MIXER" -offset indent
.Bl -tag -width "MIX_STATUS_PLAY | MIX_STATUS_REC" -offset indent .It Dv MIX_MODE_MIXER
.It Dv MIX_STATUS_NONE The audio device has a mixer.
dev_submerge.chUnsubmitted
Done
Inline Actions

Repetitive, I know, but I'd prefer "audio device" here (and below) for clarity - there's also the mixer device, right?

dev_submerge.ch: Repetitive, I know, but I'd prefer "audio device" here (and below) for clarity - there's also…
christosAuthorUnsubmitted
Done
Inline Actions

No objection, although in this specific sentence it's clear that we refer to the audio device. Will update the diff now.

christos: No objection, although in this specific sentence it's clear that we refer to the audio device.
Neither playback nor recording. .It Dv MIX_MODE_PLAY
.It Dv MIX_STATUS_PLAY The audio device supports playback.
Playback. .It Dv MIX_MODE_REC
.It Dv MIX_STATUS_REC The audio device supports recording.
Recording.
.It Dv MIX_STATUS_PLAY | MIX_STATUS_REC
Playback and recording.
.El .El
.Pp .Pp
The The
.Fn mixer_get_nmixers .Fn mixer_get_nmixers
function returns the maximum mixer unit number. function returns the maximum mixer unit number.
Although this might sound as incorrect behavior, given that one would expect Although this might sound as incorrect behavior, given that one would expect
"nmixers" to refer to the total number of active mixers, it is more intuitive "nmixers" to refer to the total number of active mixers, it is more intuitive
for applications that want to loop through all mixer devices (see the for applications that want to loop through all mixer devices (see the
▲ Show 20 Lines • Show All 183 Lines • Show Last 20 Lines
View Options

sys/netinet/tcp_subr.c

Show All 13 Lines
.\" THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR .\" THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
.\" IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, .\" IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
.\" FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE .\" FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
.\" AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER .\" AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
.\" LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, .\" LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
.\" OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN .\" OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
.\" THE SOFTWARE. .\" THE SOFTWARE.
.\" .\"
.Dd May 22, 2024 .Dd August 4, 2024
.Dt MIXER 3 .Dt MIXER 3
.Os .Os
.Sh NAME .Sh NAME
.Nm mixer_open , .Nm mixer_open ,
.Nm mixer_close , .Nm mixer_close ,
.Nm mixer_get_dev , .Nm mixer_get_dev ,
.Nm mixer_get_dev_byname , .Nm mixer_get_dev_byname ,
.Nm mixer_add_ctl , .Nm mixer_add_ctl ,
▲ Show 20 Lines • Show All 352 Lines • ▼ Show 20 Lines
functions get and set the default audio card in the system. functions get and set the default audio card in the system.
Although this is not really a mixer feature, it is useful to have instead of \ Although this is not really a mixer feature, it is useful to have instead of \
having to use the having to use the
.Xr sysctl 3 .Xr sysctl 3
controls. controls.
.Pp .Pp
The The
.Fn mixer_get_mode .Fn mixer_get_mode
function returns the playback/recording mode of the audio device the mixer \ function returns the operating mode of the audio device the mixer belongs to.
belongs to. The following values can be OR'ed in case more than one mode is supported:
dev_submerge.chUnsubmitted
Done
Inline Actions

"They"?

dev_submerge.ch: "They"?
christosAuthorUnsubmitted
Done
Inline Actions

Typo.

christos: Typo.
The available values are the following: .Bl -tag -width "MIX_MODE_MIXER" -offset indent
.Bl -tag -width "MIX_STATUS_PLAY | MIX_STATUS_REC" -offset indent .It Dv MIX_MODE_MIXER
.It Dv MIX_STATUS_NONE The audio device has a mixer.
dev_submerge.chUnsubmitted
Done
Inline Actions

Repetitive, I know, but I'd prefer "audio device" here (and below) for clarity - there's also the mixer device, right?

dev_submerge.ch: Repetitive, I know, but I'd prefer "audio device" here (and below) for clarity - there's also…
christosAuthorUnsubmitted
Done
Inline Actions

No objection, although in this specific sentence it's clear that we refer to the audio device. Will update the diff now.

christos: No objection, although in this specific sentence it's clear that we refer to the audio device.
Neither playback nor recording. .It Dv MIX_MODE_PLAY
.It Dv MIX_STATUS_PLAY The audio device supports playback.
Playback. .It Dv MIX_MODE_REC
.It Dv MIX_STATUS_REC The audio device supports recording.
Recording.
.It Dv MIX_STATUS_PLAY | MIX_STATUS_REC
Playback and recording.
.El .El
.Pp .Pp
The The
.Fn mixer_get_nmixers .Fn mixer_get_nmixers
function returns the maximum mixer unit number. function returns the maximum mixer unit number.
Although this might sound as incorrect behavior, given that one would expect Although this might sound as incorrect behavior, given that one would expect
"nmixers" to refer to the total number of active mixers, it is more intuitive "nmixers" to refer to the total number of active mixers, it is more intuitive
for applications that want to loop through all mixer devices (see the for applications that want to loop through all mixer devices (see the
▲ Show 20 Lines • Show All 183 Lines • Show Last 20 Lines