Re: [PATCH] dm cache: reject oversized invalidate_cblocks values

On Wed, Jun 10, 2026 at 7:51 AM Ming Hung Tsai <[email protected]> wrote:
>
> On Tue, Jun 9, 2026 at 8:26 AM Samuel Moelius
> <[email protected]> wrote:
> >
> > invalidate_cblocks parses the cache block number into an unsigned long
> > long, but then stores it in a narrower cblock_t.  Values that do not fit
> > in the target type are silently truncated before the cache block is
> > invalidated.
> >
> > That can make a request for one cache block invalidate a different
> > block.  It also makes the userspace-visible control file accept input
> > that cannot be represented by the mapping it is about to update.
> >
> > Reject cache block numbers that do not round-trip through cblock_t
> > before using them.
> >
> > Assisted-by: Codex:gpt-5.5-cyber-preview
> > Signed-off-by: Samuel Moelius <[email protected]>
> > ---
> >  drivers/md/dm-cache-target.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> > index 097315a9bf0f..5f1a019a9a8d 100644
> > --- a/drivers/md/dm-cache-target.c
> > +++ b/drivers/md/dm-cache-target.c
> > @@ -3330,6 +3330,9 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> >         r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
> >
> >         if (r == 2) {
> > +               if (b > U32_MAX || e > U32_MAX)
> > +                       return -EINVAL;
> > +
> >                 result->begin = to_cblock(b);
> >                 result->end = to_cblock(e);
> >                 return 0;
> > @@ -3341,8 +3344,11 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> >         r = sscanf(str, "%llu%c", &b, &dummy);
> >
> >         if (r == 1) {
> > +               if (b > U32_MAX || b == U32_MAX)
> > +                       return -EINVAL;
> > +
> >                 result->begin = to_cblock(b);
> > -               result->end = to_cblock(from_cblock(result->begin) + 1u);
> > +               result->end = to_cblock(b + 1);
> >                 return 0;
> >         }
> >
> > --
> > 2.43.0
> >
> >
>
> This patch is a half-measure. sscanf() silently wraps on overflow, so
> values beyond U64_MAX still get wrapped to a u64 value, bypass the
> range check, and invalidate a different block than specified.
>
> Also, in the single-value path, the b == U32_MAX check is redundant.
> With the original end calculation, end wraps to 0 via uint32_t
> overflow, and validate_cblock_range() already rejects it since begin
> >= end. Neither the extra check nor the new end calculation is needed
> here.
>
> To properly fix truncation, a rework using kstrtoull() is needed.

I will submit a new patch with the subject line "dm cache: parse
invalidate_cblocks with kstrtoull()".

Please consider this patch withdrawn.