Re: [PATCH] zram: fix idle age_sec underflow in idle_store()

[Hao Jia <jiahao.kernel@gmail.com>]

On 2026/8/28 14:02, Sergey Senozhatsky wrote:
> On (26/08/28 13:23), Sergey Senozhatsky wrote:
>> On (26/08/25 17:23), Hao Jia wrote:
>>> After commit 2e8ff2f51dde ("zram: use u32 for entry ac_time tracking"),
>>> idle_store() computes the idle cutoff as follows:
>>>
>>> 	cutoff = ktime_sub((u32)ktime_get_boottime_seconds(), age_sec);
>>>
>>> Because the left operand is cast to a 32-bit unsigned integer, when
>>> age_sec exceeds the current uptime, the subtraction wraps modulo 2^32
>>> and the huge result is zero-extended into the s64 cutoff.  mark_idle()
>>> subsequently marks every entry as idle instead of matching nothing,
>>> breaking the intended semantics of /sys/block/zramX/idle.  For instance,
>>> running echo 86400 > idle on a machine up for only two minutes causes
>>> all newly written pages to be marked as idle and handed over to idle
>>> writeback and recompression.
>>>
>>> Drop the explicit cast to perform the subtraction in signed arithmetic
>>> again, restoring the previous behavior: an age_sec greater than uptime
>>> yields a negative cutoff, and one equal to uptime yields 0. Since a
>>> cutoff of 0 is now a valid computed value, it can no longer double as
>>> the "all" sentinel. Switch to KTIME_MIN instead, which no ac_time can
>>> be after.
>>
>> Can we instead just reject such cases?   It should be an error to
>> request writeback of pages that are 2y old on a system that has
>> uptime of 2d.
>>
>> Maybe something like this:
>>
>> +             time64_t uptime;
>>
>> -             if (IS_ENABLED(CONFIG_ZRAM_TRACK_ENTRY_ACTIME) &&
>> -                 !kstrtouint(buf, 0, &age_sec))
>> -                     cutoff = ktime_sub((u32)ktime_get_boottime_seconds(),
>> -                                        age_sec);
>> -             else
>> +             if (!IS_ENABLED(CONFIG_ZRAM_TRACK_ENTRY_ACTIME) ||
>> +                 kstrtouint(buf, 0, &age_sec))
>>                        return -EINVAL;
>> +
>> +             /* No slot can be older than the system uptime */
>> +             uptime = ktime_get_boottime_seconds();
>> +             if (age_sec >= uptime)
>> +                     return -ERANGE;
> 
> Or perhaps, instead "return len" and make it a fast path out, w/o the
> need to iterate any slots, take locks, etc.

Thanks for the feedback! I've sent out v2 of the patch. Please take a 
look when you get a chance.

https://lore.kernel.org/all/20260828083149.45760-1-jiahao.kernel@gmail.com

Thanks,
Hao
> 
>> +
>> +             cutoff = uptime - age_sec;
>>        }