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

[Hao Jia]

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@xxxxxxxxx

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