On Thu, Apr 15, 2021 at 5:47 PM Gong, Sishuai <sish...@purdue.edu> wrote: > > Hi, > > We found a data race between tcp_set_default_congestion_control() and > tcp_set_congestion_control() in linux-5.12-rc3. > In general, when tcp_set_congestion_control() is reading ca->flags with a > lock grabbed, tcp_set_default_congestion_control() > may be updating ca->flags at the same time, as shown below. > > When the writer and reader are running parallel, > tcp_set_congestion_control()’s control flow > might be non-deterministic, either returning a -EPERM or calling > tcp_reinit_congestion_control(). > > We also notice in tcp_set_allowed_congestion_control(), the write to > ca->flags is protected by tcp_cong_list_lock, > so we want to point it out in case the data race is unexpected. > > Thread 1 Thread 2 > //tcp_set_default_congestion_control() //tcp_set_congestion_control() > // > lock_sock() grabbed > if > (!((ca->flags & TCP_CONG_NON_RESTRICTED) || cap_net_admin)) > err = > -EPERM; > else if > (!bpf_try_module_get(ca, ca->owner)) > err = > -EBUSY; > else > > tcp_reinit_congestion_control(sk, ca); > ca->flags |= TCP_CONG_NON_RESTRICTED; > > > > Thanks, > Sishuai >
Yes, obviously reading ca->flags while another thread might set the bit is racy. This is of no consequence, if you want to silence KCSAN please a patch.
