linux-kernel - Re: [PATCH 2/3] mm/tlb: Fix wrong judgement in allocate_global

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250331051234.GB88569@k08j02272.eu95sqa>
Date: Mon, 31 Mar 2025 13:12:34 +0800
From: "Hou Wenlong" <houwenlong.hwl@...group.com>
To: Rik van Riel <riel@...riel.com>
Cc: linux-kernel@...r.kernel.org, Dave Hansen <dave.hansen@...ux.intel.com>,
	Andy Lutomirski <luto@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH 2/3] mm/tlb: Fix wrong judgement in allocate_global_asid()

On Sun, Mar 30, 2025 at 01:33:44AM +0800, Rik van Riel wrote:
> On Sat, 2025-03-29 at 21:05 +0800, Hou Wenlong wrote:
> > In allocate_global_asid(), 'global_asid_available' cannot be zero, as
> > it
> > has already been checked in use_global_asid(). Therefore, the warning
> > in
> > allocate_global_asid() cannot be triggered; fix the wrong judgment in
> > allocate_global_asid().
> > 
> > Fixes: d504d1247e36 ("x86/mm: Add global ASID allocation helper
> > functions")
> > Signed-off-by: Hou Wenlong <houwenlong.hwl@...group.com>
> 
> Good catch.
> 
> Reviewed-by: Rik van Riel <riel@...riel.com>
> 
> 
> Looking at allocate_global_asid() again, I wonder if
> that needs to be turned back into a loop.
> 
> What if we have no global asids available, and then
> an asid gets freed that is smaller than the value
> of last_global_asid?

Uh, I think this can be an easily triggered issue in the current code.
Moreover, reset_global_asid_space() is only called when 'last_global_asid'
is 'MAX_ASID_AVAILALBE-1', which means that free ASIDs are returned to
avaialable pool only in that case. So if the 'global_asid_used'
bitmap is full, but 'last_global_asid' is not 'MAX_ASID_AVIALBLE-1', the
allocation can fail as well. So I think the allocation may look like
this:
```
index 0f86c3140fdc..7f4bcb0e3d8c 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -312,17 +312,27 @@ static u16 allocate_global_asid(void)

        lockdep_assert_held(&global_asid_lock);

-       /* The previous allocation hit the edge of available address space */
-       if (last_global_asid >= MAX_ASID_AVAILABLE - 1)
-               reset_global_asid_space();
-
+       /* Search the range [last_global_asid, MAX_ASID_AVAILABLE-1]. */
        asid = find_next_zero_bit(global_asid_used, MAX_ASID_AVAILABLE, last_global_asid);
-
-       if (asid >= MAX_ASID_AVAILABLE && global_asid_available) {
-               /* This should never happen. */
-               VM_WARN_ONCE(1, "Unable to allocate global ASID despite %d available\n",
-                               global_asid_available);
-               return 0;
+       if (asid >= MAX_ASID_AVAILABLE) {
+               /* Search the range [TLB_NR_DYN_ASIDS, last_global_asid-1]. */
+               asid = find_next_zero_bit(global_asid_used, last_global_asid, TLB_NR_DYN_ASIDS);
+               if (asid >= last_global_asid) {
+                       /*
+                        * The 'global_asid_used' bitmap is full, so merge the
+                        * 'global_asid_freed' bitmap and search from the
+                        * beginning again.
+                        */
+                       reset_global_asid_space();
+                       asid = find_next_zero_bit(global_asid_used, MAX_ASID_AVAILABLE,
+                                                 last_global_asid);
+                       if (asid >= MAX_ASID_AVAILABLE && global_asid_available) {
+                               /* This should never happen. */
+                               VM_WARN_ONCE(1, "Unable to allocate global ASID despite %d available\n",
+                                               global_asid_available);
+                               return 0;
+                       }
+               }
        }
```

> -- 
> All Rights Reversed.