On 8/24/21 12:02, Matthew Wilcox wrote:
> On Wed, Aug 18, 2021 at 05:22:39PM +0200, Vlastimil Babka wrote:
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 403a175a720f..ef3554314b47 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -936,6 +936,7 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
>> void drop_slab_node(int nid)
>> {
>> unsigned long freed;
>> + int shift = 0;
>>
>> do {
>> struct mem_cgroup *memcg = NULL;
>> @@ -948,7 +949,7 @@ void drop_slab_node(int nid)
>> do {
>> freed += shrink_slab(GFP_KERNEL, nid, memcg, 0);
>> } while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)) != NULL);
>> - } while (freed > 10);
>> + } while ((freed >> shift++) > 0);
>
> This can, if you're really unlucky, produce UB. If you free 2^63 items
> when shift is 63, then 2^63 >> 63 is 1 and shift becomes 64, producing
> UB on the next iteration. We could do:
>
> } while (shift < BITS_PER_LONG) && (freed >> shift++) > 0);
>
> but honestly, that feels silly. How about:
>
> } while ((freed >> shift++) > 1);
>
> almost exactly as arbitrary, but guarantees no UB.
Hey, zero is not arbitrary :P
But thanks, here's a fix up.
>From 88189bf16406c5910400193422b3f18f859f18d8 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@xxxxxxx>
Date: Tue, 24 Aug 2021 14:08:53 +0200
Subject: [PATCH] mm, vmscan: guarantee drop_slab_node() termination-fix
Matthew reports [1] that if we free enough objects, we can eventually
right-shift by BITS_PER_LONG, which is undefined behavior. Raise the
threshold from 0 to 1 which means we will shift only up to BITS_PER_LONG-1.
[1] https://lore.kernel.org/linux-mm/YSTDnqKgQLvziyQI@xxxxxxxxxxxxxxxxxxxx/
Reported-by: Matthew Wilcox <willy@xxxxxxxxxxxxx>
Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
---
mm/vmscan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4ffaa7970904..f08aef08c351 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -952,7 +952,7 @@ void drop_slab_node(int nid)
do {
freed += shrink_slab(GFP_KERNEL, nid, memcg, 0);
} while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)) != NULL);
- } while ((freed >> shift++) > 0);
+ } while ((freed >> shift++) > 1);
}
void drop_slab(void)
--
2.32.0
