Received-SPF: pass (google.com: domain of linux-kernel+bounces-9232-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <05729f32-44ec-416d-963d-5218ca890fb8@arista.com>
Date: Fri, 22 Dec 2023 01:17:48 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v16 net-next 01/23] net/tcp: Prepare tcp_md5sig_pool for
 TCP-AO
Content-Language: en-US
To: Eric Dumazet <edumazet@google.com>
Cc: David Ahern <dsahern@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Jakub Kicinski <kuba@kernel.org>, "David S. Miller" <davem@davemloft.net>,
 linux-kernel@vger.kernel.org, Andy Lutomirski <luto@amacapital.net>,
 Ard Biesheuvel <ardb@kernel.org>, Bob Gilligan <gilligan@arista.com>,
 Dan Carpenter <error27@gmail.com>, David Laight <David.Laight@aculab.com>,
 Dmitry Safonov <0x7f454c46@gmail.com>, Donald Cassidy <dcassidy@redhat.com>,
 Eric Biggers <ebiggers@kernel.org>, "Eric W. Biederman"
 <ebiederm@xmission.com>, Francesco Ruggeri <fruggeri05@gmail.com>,
 "Gaillardetz, Dominik" <dgaillar@ciena.com>,
 Herbert Xu <herbert@gondor.apana.org.au>,
 Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
 Ivan Delalande <colona@arista.com>, Leonard Crestez <cdleonard@gmail.com>,
 "Nassiri, Mohammad" <mnassiri@ciena.com>,
 Salam Noureddine <noureddine@arista.com>, Simon Horman <horms@kernel.org>,
 "Tetreault, Francois" <ftetreau@ciena.com>, netdev@vger.kernel.org,
 Steen Hegelund <Steen.Hegelund@microchip.com>
References: <20231023192217.426455-1-dima@arista.com>
 <20231023192217.426455-2-dima@arista.com>
 <CANn89i+Uwg87xAS9m8fm1f1daQj-YyugperN3HnvgbB6g+hOuw@mail.gmail.com>
From: Dmitry Safonov <dima@arista.com>
In-Reply-To: <CANn89i+Uwg87xAS9m8fm1f1daQj-YyugperN3HnvgbB6g+hOuw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Hi Eric,

On 12/21/23 14:31, Eric Dumazet wrote:
> On Mon, Oct 23, 2023 at 9:22 PM Dmitry Safonov <dima@arista.com> wrote:
...
>> +int tcp_sigpool_alloc_ahash(const char *alg, size_t scratch_size)
>> +{
>> +       int i, ret;
>> +
>> +       /* slow-path */
>> +       mutex_lock(&cpool_mutex);
>> +       ret = sigpool_reserve_scratch(scratch_size);
>> +       if (ret)
>> +               goto out;
>> +       for (i = 0; i < cpool_populated; i++) {
>> +               if (!cpool[i].alg)
>> +                       continue;
>> +               if (strcmp(cpool[i].alg, alg))
>> +                       continue;
>> +
>> +               if (kref_read(&cpool[i].kref) > 0)
>> +                       kref_get(&cpool[i].kref);
> 
> This sequence is racy.
> 
> You must use kref_get_unless_zero().

Thanks for this report, I've sent a trivial fixup here:
https://lore.kernel.org/all/20231222-tcp-ao-kref_get_unless_zero-v1-1-551c2edd0136@arista.com/T/#u

> 
>> +               else
>> +                       kref_init(&cpool[i].kref);
>> +               ret = i;
>> +               goto out;
>> +       }
>> +
>> +
> 
> syzbot reported:
> 
> refcount_t: addition on 0; use-after-free.
> WARNING: CPU: 2 PID: 31702 at lib/refcount.c:25
> refcount_warn_saturate+0x1ca/0x210 lib/refcount.c:25
> Modules linked in:
> CPU: 2 PID: 31702 Comm: syz-executor.3 Not tainted
> 6.7.0-rc6-syzkaller-00044-g1a44b0073b92 #0
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> 1.16.2-debian-1.16.2-1 04/01/2014
> RIP: 0010:refcount_warn_saturate+0x1ca/0x210 lib/refcount.c:25
> Code: ff 89 de e8 58 a3 25 fd 84 db 0f 85 e6 fe ff ff e8 1b a8 25 fd
> c6 05 9a 88 a1 0a 01 90 48 c7 c7 00 9d 2e 8b e8 b7 ec eb fc 90 <0f> 0b
> 90 90 e9 c3 fe ff ff e8 f8 a7 25 fd c6 05 75 88 a1 0a 01 90
> RSP: 0018:ffffc900296df850 EFLAGS: 00010286
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffc9002c40a000
> RDX: 0000000000040000 RSI: ffffffff814db526 RDI: 0000000000000001
> RBP: ffffffff92b5b7b0 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000001 R11: 0000000000000002 R12: 0000000000000010
> R13: ffffffff92b5b7b0 R14: 0000000000000001 R15: 0000000000000000
> FS: 0000000000000000(0000) GS:ffff88802c800000(0063) knlGS:00000000f7efdb40
> CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
> CR2: 00000000f7354000 CR3: 0000000050ee3000 CR4: 0000000000350ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> __refcount_add include/linux/refcount.h:199 [inline]
> __refcount_inc include/linux/refcount.h:250 [inline]
> refcount_inc include/linux/refcount.h:267 [inline]
> kref_get include/linux/kref.h:45 [inline]
> tcp_sigpool_alloc_ahash+0x9cb/0xce0 net/ipv4/tcp_sigpool.c:166
> tcp_md5_alloc_sigpool+0x1b/0x40 net/ipv4/tcp.c:4379
> tcp_md5_do_add+0x192/0x460 net/ipv4/tcp_ipv4.c:1403
> tcp_v6_parse_md5_keys+0x68d/0x860 net/ipv6/tcp_ipv6.c:676
> do_tcp_setsockopt+0x1302/0x2880 net/ipv4/tcp.c:3644
> tcp_setsockopt+0xd4/0x100 net/ipv4/tcp.c:3726
> do_sock_setsockopt+0x222/0x470 net/socket.c:2311
> __sys_setsockopt+0x1a6/0x270 net/socket.c:2334
> __do_sys_setsockopt net/socket.c:2343 [inline]
> __se_sys_setsockopt net/socket.c:2340 [inline]
> __ia32_sys_setsockopt+0xbc/0x150 net/socket.c:2340
> do_syscall_32_irqs_on arch/x86/entry/common.c:165 [inline]
> __do_fast_syscall_32+0x62/0xe0 arch/x86/entry/common.c:321
> do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:346
> entry_SYSENTER_compat_after_hwframe+0x70/0x7a
> RIP: 0023:0xf7f02579
> Code: b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00
> 00 00 00 00 00 00 00 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a
> 59 c3 90 90 90 90 8d b4 26 00 00 00 00 8d b4 26 00 00 00 00
> RSP: 002b:00000000f7efd5ac EFLAGS: 00000292 ORIG_RAX: 000000000000016e
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000000006
> RDX: 000000000000000e RSI: 0000000020000000 RDI: 00000000000000d8
> RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000292 R12: 0000000000000000
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000

Thanks,
            Dmitry