From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id AB5C0C48BF6 for ; Sat, 24 Feb 2024 11:39:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1844A6B006E; Sat, 24 Feb 2024 06:39:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1352E6B0071; Sat, 24 Feb 2024 06:39:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F3ECF6B0072; Sat, 24 Feb 2024 06:39:01 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id E4F686B006E for ; Sat, 24 Feb 2024 06:39:01 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 335A916027C for ; Sat, 24 Feb 2024 11:39:01 +0000 (UTC) X-FDA: 81826500882.14.78E858F Received: from www262.sakura.ne.jp (www262.sakura.ne.jp [202.181.97.72]) by imf26.hostedemail.com (Postfix) with ESMTP id 3ABAF14000F for ; Sat, 24 Feb 2024 11:38:57 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf26.hostedemail.com: domain of penguin-kernel@I-love.SAKURA.ne.jp designates 202.181.97.72 as permitted sender) smtp.mailfrom=penguin-kernel@I-love.SAKURA.ne.jp ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708774739; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Yy98UP6s8Jpm2h5SgCfsqqr1/gOjzT7uZuHyOPKTafA=; b=Ca+jlrvtXh9iZEU4P2Td7wsJUUt2SCAPXwacilTI95IqhCvh4czuNOz9E+fcDduQNKVxBQ xA3o3/5hLXphHD5mjACy4KX0uZCvtuQ4N4KBJSrs8qUebwssZL3B+W8ewuHdTZBBj77hnO sjxMZaq4ONSR3tdFpqcD4BuuqA/CAMw= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf26.hostedemail.com: domain of penguin-kernel@I-love.SAKURA.ne.jp designates 202.181.97.72 as permitted sender) smtp.mailfrom=penguin-kernel@I-love.SAKURA.ne.jp ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708774739; a=rsa-sha256; cv=none; b=cuxPEmKpLNhduYJBhs2iR1C02Rm/SHb06IoV42m+ktqdq7+o9Do52ZdU0qRenPDh3Lb79+ mEMGWbyHIgkqz/lNPiPHP2C+q8n9GHhuFHourO925z/pZ7D6AqHZGTZj7Zox/kSwOQS6MJ JC2UEm+KHNR4nNmSyUyslFwS03BqnKg= Received: from fsav311.sakura.ne.jp (fsav311.sakura.ne.jp [153.120.85.142]) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTP id 41OBcdWr066959; Sat, 24 Feb 2024 20:38:39 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Received: from www262.sakura.ne.jp (202.181.97.72) by fsav311.sakura.ne.jp (F-Secure/fsigk_smtp/550/fsav311.sakura.ne.jp); Sat, 24 Feb 2024 20:38:39 +0900 (JST) X-Virus-Status: clean(F-Secure/fsigk_smtp/550/fsav311.sakura.ne.jp) Received: from [192.168.1.6] (M106072142033.v4.enabler.ne.jp [106.72.142.33]) (authenticated bits=0) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTPSA id 41OBcdAu066956 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NO); Sat, 24 Feb 2024 20:38:39 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Message-ID: Date: Sat, 24 Feb 2024 20:38:37 +0900 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/2] stackdepot: make fast paths lock-less again Content-Language: en-US To: Marco Elver Cc: Andrey Konovalov , Alexander Potapenko , Dmitry Vyukov , Vlastimil Babka , linux-mm@kvack.org, kasan-dev@googlegroups.com, Andi Kleen , Andrew Morton References: <20240118110216.2539519-1-elver@google.com> <20240118110216.2539519-2-elver@google.com> From: Tetsuo Handa In-Reply-To: <20240118110216.2539519-2-elver@google.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 3ABAF14000F X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: ox5pqn8opxuqmmhiqc18mbieuuaojwue X-HE-Tag: 1708774737-4130 X-HE-Meta: U2FsdGVkX18f/cHwY6TLLmWf5zjFJKQUNlQP8b+BDngIDMdpTkqoTWGmBiFlQE2RWxyzbF9B4kuFAaydT6gj41niv0m2UFcqB8zniDzea54W/VxHGk40dSBkDUCYfGNuzgSJa3vtbmr8v1ytKIU6jww9/ySDpSnhgvUdER3gKKqTQr1LUP3i+mtb94O54S30ofiXfKwG4zq/a5nC1kROvexxSlQq+tddQL9ZE4zYSW179vLW0J+KK7qVgcZsEKBUXY6j76VkUBbH7LruzVMxdJzCCXzsndaxyFYUqC3FFbRXE5fUguWOQz4Kf3g9xqm4VVGs1RZJQmgPbIkX2OkftNRSggfCEncjWXuP5MFvj5ax1I+n5+ImXsxng1CM7KvbnUjzE0MFoyZFZzsxWtFlvgOowu2WwvmYIx6jxZZu1E68PK+4tjYg6kFIjnlejxyCssgSwVlWVy4KNgKc4hnp0DqCYQdP0DUNDgxHGc4hTRIhwTMeI/FYTRXBg5mo3qJCxO+L1a79Pt7Rl6VcSnnARiBX4aKlUmPvljhoq08GrQQnoXwuQaW5cwdbYEaWy89A41kSgLj0L0iMoVlDG8hj9EkixkQxvxAI52fwnAStC7pP884aycgg/aO51Hd/0xXDWQihICtQDTUEcl3OBcIWb55V6ABox/ugMKiW3SuaCAZ/nlbRHVXgwXzyg+IVRU+NhwK+2K7N2eBgTCFEDGoGZ9H54N+fuzW6HjsCaXt3+V2okWzZkhL0aPmFsClKPPfBhEl9V1yQr5udjKc9/ZytKS4AK1n1L4qflQPc8FivH9pyAgSdRNICtqLThf5Ff8vCY+7dPjot1jfqJ3y/mF99a16EAyIc+if+fzN8sTQ74xmvw9fg8Fa4Tpwb60ABD9WGUlNk1z/MhuqN+05sm1iZ7y2KKFkKoJQPAj9uNahjnKOkdS5IynjSbeK5MnmS3CQ/X8EPXDz246Fe1z807jN V6Qnmw/n 1+b/lMwg510lwa5JuIPPYJWmUPiBwsjZb0Idv587+vHNv3V7XhJ4L5EjzR/n9GfGiCn/sT6O42H9AquqtJPaPKHvC8HwFEJHRO6ZqtUCjq+luJ0qIUdKsDzqA1WhuJ6miyNYEd5lp9WujuE/3NyezHcxhZ2s3Y0mOfgd18B3hCP0crMR8cJB2o/Z6kcOupM+tKzHKxS9Lqn3/vx8pRP0+tYlHEk2SnLOwudAhTrjRn/uzTApSrF23K6DlemPe7KrsXm3nZ6aIlGt2LAAvGw2y3x+wdo27kPwh4uiy6NrrqMAicjRm9v3asXGu6Q== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hello, stackdepot developers. I suspect that commit 4434a56ec209 ("stackdepot: make fast paths lock-less again") is not safe, for https://syzkaller.appspot.com/x/error.txt?x=1409f29a180000 is reporting UAF at list_del() below from stack_depot_save_flags(). ---------- + /* + * We maintain the invariant that the elements in front are least + * recently used, and are therefore more likely to be associated with an + * RCU grace period in the past. Consequently it is sufficient to only + * check the first entry. + */ + stack = list_first_entry(&free_stacks, struct stack_record, free_list); + if (stack->size && !poll_state_synchronize_rcu(stack->rcu_state)) + return NULL; + + list_del(&stack->free_list); + counters[DEPOT_COUNTER_FREELIST_SIZE]--; ---------- Commit 4434a56ec209 says that race is handled by refcount_inc_not_zero(), but refcount_inc_not_zero() is called only if STACK_DEPOT_FLAG_GET is specified. ---------- + list_for_each_entry_rcu(stack, bucket, hash_list) { + if (stack->hash != hash || stack->size != size) + continue; - lockdep_assert_held(&pool_rwlock); + /* + * This may race with depot_free_stack() accessing the freelist + * management state unioned with @entries. The refcount is zero + * in that case and the below refcount_inc_not_zero() will fail. + */ + if (data_race(stackdepot_memcmp(entries, stack->entries, size))) + continue; - list_for_each(pos, bucket) { - found = list_entry(pos, struct stack_record, list); - if (found->hash == hash && - found->size == size && - !stackdepot_memcmp(entries, found->entries, size)) - return found; + /* + * Try to increment refcount. If this succeeds, the stack record + * is valid and has not yet been freed. + * + * If STACK_DEPOT_FLAG_GET is not used, it is undefined behavior + * to then call stack_depot_put() later, and we can assume that + * a stack record is never placed back on the freelist. + */ + if ((flags & STACK_DEPOT_FLAG_GET) && !refcount_inc_not_zero(&stack->count)) + continue; + + ret = stack; + break; } ---------- I worried that if we race when STACK_DEPOT_FLAG_GET is not specified, depot_alloc_stack() by error overwrites stack->free_list via memcpy(stack->entries, ...), and invalid memory access happens when stack->free_list.next is read. Therefore, I tried https://syzkaller.appspot.com/text?tag=Patch&x=17a12a30180000 but did not help ( https://syzkaller.appspot.com/x/error.txt?x=1423a4ac180000 ). Therefore, I started to suspect how stack_depot_save() (which does not set STACK_DEPOT_FLAG_GET) can be safe. Don't all callers need to set STACK_DEPOT_FLAG_GET when calling stack_depot_save_flags() and need to call stack_depot_put() ?