grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v14 1/3] safemath: Add ALIGN_UP_OVF() that checks for {over,


From: Vladimir 'phcoder' Serbinenko
Subject: Re: [PATCH v14 1/3] safemath: Add ALIGN_UP_OVF() that checks for {over, under}flow
Date: Sat, 18 May 2024 12:20:47 +0300



Le ven. 17 mai 2024, 21:26, Gao Xiang <hsiangkao@linux.alibaba.com> a écrit :
Hi Vladimir,

On 2024/5/18 00:38, Vladimir 'phcoder' Serbinenko wrote:
> I think that given that align is a non zero const we can trust it.

 From the EROFS specific cases, they are always non-zero values,
So I agree with you on this..

Yet Daniel said ".. Be careful with underflows too." in
Zkcv7G7MJBg8sKiE@tomti.i.net-space.pl" rel="noreferrer noreferrer" target="_blank">https://lore.kernel.org/r/Zkcv7G7MJBg8sKiE@tomti.i.net-space.pl
although I'm not quite sure but I guess like this.

Also as a generic helper, I think `align` could be zero if it's
a variable..

It's rare for align to be a variable and then additional checks are needed that align is a power of 2. And power of 2 is never zero. This check is better done on the caller side. You just need a comment specify that it's caller responsibility to check it.

Anyway, either way works for EROFS.   I just try my best to do
what I could do for this first series for upstreaming and we
could move forward to the next step..

Thanks,
Gao Xiang

>
> Le ven. 17 mai 2024, 15:56, Gao Xiang <hsiangkao@linux.alibaba.com <mailto:hsiangkao@linux.alibaba.com>> a écrit :
>
>     The following EROFS patch will use this helper to handle overflow
>     ALIGN_UP() cases.
>
>     Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com <mailto:hsiangkao@linux.alibaba.com>>
>     ---
>       include/grub/safemath.h | 16 ++++++++++++++++
>       1 file changed, 16 insertions(+)
>
>     diff --git a/include/grub/safemath.h b/include/grub/safemath.h
>     index fbd9b5925..baaea0ef4 100644
>     --- a/include/grub/safemath.h
>     +++ b/include/grub/safemath.h
>     @@ -32,6 +32,22 @@
>
>       #define grub_cast(a, res)      grub_add ((a), 0, (res))
>
>     +#define ALIGN_UP_OVF(v, align, res)                    \
>     +({                                                     \
>     +  bool __failed;                                       \
>     +  typeof(v) a;                                         \
>     +                                                       \
>     +  __failed = grub_sub ((typeof(v))(align), 1, &a);     \
>     +  if (__failed == false)                               \
>     +    {                                                  \
>     +    __failed = grub_add (v, a, res);                   \
>     +    if (__failed == false)                             \
>     +      {                                                        \
>     +        *(res) &= ~a;                                  \
>     +      }                                                        \
>     +    }                                                  \
>     +__failed;})
>     +
>       #else
>       #error gcc 5.1 or newer or clang 8.0 or newer is required
>       #endif
>     --
>     2.39.3
>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]