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

From: "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com>
To: Gao Xiang <hsiangkao@linux.alibaba.com>
Cc: Daniel Kiper <dkiper@net-space.pl>,
	Glenn Washburn <development@efficientek.com>,
	 Daniel Axtens <dja@axtens.net>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	 The development of GRUB 2 <grub-devel@gnu.org>,
	Yifan Zhao <zhaoyifan@sjtu.edu.cn>
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	[thread overview]
Message-ID: <CAEaD8JNwa8W=3LvaVeDG0hByz10+XHEDwCsV8TQe5md_13s2jg@mail.gmail.com> (raw)
In-Reply-To: <8659b068-896b-4127-9da9-eda629c9c4d2@linux.alibaba.com>

[-- Attachment #1.1: Type: text/plain, Size: 2968 bytes --]

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

[-- Attachment #1.2: Type: text/html, Size: 4636 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel