[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+icZUVuRaMs=bx775gDF88_xzy8LFkBA5xaK21hFDeYvgo12A@mail.gmail.com>
Date: Mon, 13 Sep 2021 21:57:05 +0200
From: Sedat Dilek <sedat.dilek@...il.com>
To: Nick Desaulniers <ndesaulniers@...gle.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Naresh Kamboju <naresh.kamboju@...aro.org>,
Nathan Chancellor <natechancellor@...il.com>,
Baokun Li <libaokun1@...wei.com>,
open list <linux-kernel@...r.kernel.org>,
linux-stable <stable@...r.kernel.org>,
Hulk Robot <hulkci@...wei.com>,
Josef Bacik <josef@...icpanda.com>,
Jens Axboe <axboe@...nel.dk>, Sasha Levin <sashal@...nel.org>,
clang-built-linux <clang-built-linux@...glegroups.com>,
lkft-triage@...ts.linaro.org, llvm@...ts.linux.dev,
Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH 5.14 018/334] nbd: add the check to prevent overflow in __nbd_ioctl()
On Mon, Sep 13, 2021 at 9:53 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@...glegroups.com> wrote:
>
> On Mon, Sep 13, 2021 at 11:39 AM Nick Desaulniers
> <ndesaulniers@...gle.com> wrote:
> >
> > On Mon, Sep 13, 2021 at 10:58 AM Greg Kroah-Hartman
> > <gregkh@...uxfoundation.org> wrote:
> > >
> > > On Mon, Sep 13, 2021 at 09:52:33PM +0530, Naresh Kamboju wrote:
> > > > [PATCH 00/10] raise minimum GCC version to 5.1
> > > > https://lore.kernel.org/lkml/20210910234047.1019925-1-ndesaulniers@google.com/
> > >
> > > Has anyone submitted a fix for this upstream yet? I can't seem to find
> > > one :(
> >
> > That lore link has a series to address this, though that's maybe
> > something we don't want to backport to stable.
> >
> > I thought about this all weekend; I think I might be able to work
> > around the one concern I had with my other approach, using
> > __builtin_choose_expr().
> >
> > There's an issue with my alternative approach
> > (https://gist.github.com/nickdesaulniers/2479818f4983bbf2d688cebbab435863)
> > with declaring the local variable z in div_64() since either operand
> > could be 64b, which result in an unwanted truncation if the dividend
> > is 32b (or less, and divisor is 64b). I think (what I realized this
> > weekend) is that we might be able to replace the `if` with
> > `__builtin_choose_expr`, then have that whole expression be the final
> > statement and thus the "return value" of the statement expression.
>
> Christ...that...works? Though, did Linus just merge my patches for gcc 5.1?
>
"Merge branch 'gcc-min-version-5.1' (make gcc-5.1 the minimum version)"
- Sedat -
https://git.kernel.org/linus/316346243be6df12799c0b64b788e06bad97c30b
> Anyways, I'll send something like this for stable:
> ---
>
> diff --git a/include/linux/math64.h b/include/linux/math64.h
> index 2928f03d6d46..e9ab8c25f8d3 100644
> --- a/include/linux/math64.h
> +++ b/include/linux/math64.h
> @@ -11,6 +11,9 @@
>
> #define div64_long(x, y) div64_s64((x), (y))
> #define div64_ul(x, y) div64_u64((x), (y))
> +#ifndef is_signed_type
> +#define is_signed_type(type) (((type)(-1)) < (type)1)
> +#endif
>
> /**
> * div_u64_rem - unsigned 64bit divide with 32bit divisor with remainder
> @@ -112,6 +115,15 @@ extern s64 div64_s64(s64 dividend, s64 divisor);
>
> #endif /* BITS_PER_LONG */
>
> +#define div64_x64(dividend, divisor) ({ \
> + BUILD_BUG_ON_MSG(sizeof(dividend) < sizeof(u64),\
> + "prefer div_x64"); \
> + __builtin_choose_expr( \
> + is_signed_type(typeof(dividend)), \
> + div64_s64(dividend, divisor), \
> + div64_u64(dividend, divisor)); \
> +})
> +
> /**
> * div_u64 - unsigned 64bit divide with 32bit divisor
> * @dividend: unsigned 64bit dividend
> @@ -142,6 +154,28 @@ static inline s64 div_s64(s64 dividend, s32 divisor)
> }
> #endif
>
> +#define div_x64(dividend, divisor) ({ \
> + BUILD_BUG_ON_MSG(sizeof(dividend) > sizeof(u32),\
> + "prefer div64_x64"); \
> + __builtin_choose_expr( \
> + is_signed_type(typeof(dividend)), \
> + div_s64(dividend, divisor), \
> + div_u64(dividend, divisor)); \
> +})
> +
> +// TODO: what if divisor is 128b?
> +#define div_64(dividend, divisor) ({
> \
> + __builtin_choose_expr(
> \
> + __builtin_types_compatible_p(typeof(dividend), s64) ||
> \
> + __builtin_types_compatible_p(typeof(dividend), u64),
> \
> + __builtin_choose_expr(
> \
> + __builtin_types_compatible_p(typeof(divisor),
> s64) || \
> + __builtin_types_compatible_p(typeof(divisor),
> u64), \
> + div64_x64(dividend, divisor),
> \
> + div_x64(dividend, divisor)),
> \
> + dividend / divisor);
> \
> +})
> +
> u32 iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder);
>
> #ifndef mul_u32_u32
> ---
> --
> Thanks,
> ~Nick Desaulniers
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmCS5Q7AzUL5nziYVU7RrtRjoE9JjOXfVBWagO1Bzbsew%40mail.gmail.com.
Powered by blists - more mailing lists