On Fri, Jul 2, 2021 at 3:41 PM Nguyen Dinh Phi <phind.uet@xxxxxxxxx> wrote: > > This commit fixes a bug (found by syzkaller) that could cause spurious > double-initializations for congestion control modules, which could cause > memory leaks orother problems for congestion control modules (like CDG) > that allocate memory in their init functions. > > The buggy scenario constructed by syzkaller was something like: > > (1) create a TCP socket > (2) initiate a TFO connect via sendto() > (3) while socket is in TCP_SYN_SENT, call setsockopt(TCP_CONGESTION), > which calls: > tcp_set_congestion_control() -> > tcp_reinit_congestion_control() -> > tcp_init_congestion_control() > (4) receive ACK, connection is established, call tcp_init_transfer(), > set icsk_ca_initialized=0 (without first calling cc->release()), > call tcp_init_congestion_control() again. > > Note that in this sequence tcp_init_congestion_control() is called > twice without a cc->release() call in between. Thus, for CC modules > that allocate memory in their init() function, e.g, CDG, a memory leak > may occur. The syzkaller tool managed to find a reproducer that > triggered such a leak in CDG. > > The bug was introduced when that commit 8919a9b31eb4 ("tcp: Only init > congestion control if not initialized already") > introduced icsk_ca_initialized and set icsk_ca_initialized to 0 in > tcp_init_transfer(), missing the possibility for a sequence like the > one above, where a process could call setsockopt(TCP_CONGESTION) in > state TCP_SYN_SENT (i.e. after the connect() or TFO open sendmsg()), > which would call tcp_init_congestion_control(). It did not intend to > reset any initialization that the user had already explicitly made; > it just missed the possibility of that particular sequence (which > syzkaller managed to find). > > Fixes: commit 8919a9b31eb4 ("tcp: Only init congestion control if not > initialized already") Please note that the patchwork tools have found a style/formatting issue with your Fixes tag: You can find them at: https://patchwork.kernel.org/project/netdevbpf/list/ -> https://patchwork.kernel.org/project/netdevbpf/patch/20210702194033.1370634-1-phind.uet@xxxxxxxxx/ -> https://patchwork.hopto.org/static/nipa/510221/12356435/verify_fixes/stdout The error is: --- Fixes tag: Fixes: commit 8919a9b31eb4 ("tcp: Only init congestion control if not Has these problem(s): - leading word 'commit' unexpected - Subject has leading but no trailing parentheses - Subject has leading but no trailing quotes --- Basically, please omit the "commit" and don't wrap the text (it's OK if it's longer than 80 or 100 characters). thanks, neal