Re: [PATCH v2 0/4] fix unexpected type conversions and potential overflows

[Qi Zheng <qi.zheng@linux.dev>]

On 3/26/26 7:57 AM, Andrew Morton wrote:
> On Wed, 25 Mar 2026 22:13:21 +0800 Qi Zheng <qi.zheng@linux.dev> wrote:
> 
>> As Harry Yoo pointed out [1], in scenarios where massive state updates occur
>> (e.g., during the reparenting of LRU folios), the values passed to memcg stat
>> update functions can accumulate and exceed the upper limit of a 32-bit integer.
>>
>> If the parameter types are not large enough (like 'int') or are handled
>> incorrectly, it can lead to severe truncation, potential overflow issues,
>> and unexpected type conversion bugs.
>>
>> This series aims to address these issues by correcting the parameter types
>> in the relevant functions, and fixing an implicit conversion bug in
>> memcg_state_val_in_pages().
> 
> Thanks. I'll add this to mm.git's mm-new branch.
> 
> AI review
> (https://sashiko.dev/#/patchset/cover.1774447069.git.zhengqi.arch%40bytedance.com)
> still points at the problem in [2/4], now describing it as a bisection
> hole.
> 
> In
> https://lkml.kernel.org/r/5fed0611-434c-4fd4-956c-39f23e0459a1@linux.dev
> you said you were going to address this by using abs(), and I see that
> being done in [4/4] so yup, runtime bisection hole.

Right.

> 
> I'm inclined to mark this "don't care".  But if we decide to backport
> [2/4] ("to prevent potential overflow issues") then we might have a
> problem.
> 
> Also, if some downstream person decides to backport [2/4] into their
> kernel without [4/4] then they'll have a bad day.

As Harry Yoo pointed out:

```
Let's look at an example (assuming unit is 1).

val = val * unit = -16384 (-16 KiB)
val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
max(0x3FFFFFFFFFFFFF, 1UL) = 0x3FFFFFFFFFF

Yeah, that's a massive positive number.

Hmm but how did it work when it was int?

val = val * unit = -16384 (-16KiB)
val * unit / PAGE_SIZE = 0xFFFFFFFFFFFFC000 / PAGE_SIZE = 0x3FFFFFFFFFFFFF
max(val * unit / PAGE_SIZE, 1UL) = 0x3FFFFFFFFFFFFF
(int)0x3FFFFFFFFFFFFF = 0xFFFFFFFF = (-1)

That's incorrect. It should have been -4?
```

this can already be an issue even without [2/4], and
[2/4] just amplify it.

Before LRU folio reparenting was introduced, we wouldn’t pass in such a
large value, so this wasn’t a problem. Since LRU folio reparenting is
still in mm-unstable, so I didn't add a Fixes tag in [4/4].

> 
> So perhaps this issue should be addressed within [2/4]?

Because they fix different existing problems, I previously split them
into two patches.

However, since the issues are all quite minor, I feel that combining
them into a single patch is fine.

 >> Actually I can trivially do this locally if you like - just turn [4/4]
i>> nto a -fix patch against [2/4], squash them together later on.
 >>
 >> Please lmk if you'd like me to do that.

That's fine, thank you!