From: David Laight <david.laight.linux@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: "Romain Gantois" <romain.gantois@bootlin.com>,
"Jonathan Cameron" <jic23@kernel.org>,
"David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Hans de Goede" <hansg@kernel.org>,
"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
"Jonathan Cameron" <Jonathan.Cameron@huawei.com>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH] iio: inkern: Avoid risky abs() usage in iio_multiply_value()
Date: Tue, 31 Mar 2026 23:04:51 +0100 [thread overview]
Message-ID: <20260331230451.6c0bd155@pumpkin> (raw)
In-Reply-To: <acwTnoz0aFs_xCyO@ashevche-desk.local>
On Tue, 31 Mar 2026 21:34:06 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Tue, Mar 31, 2026 at 04:26:35PM +0100, David Laight wrote:
> > On Tue, 31 Mar 2026 12:29:22 +0300
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > On Tue, Mar 31, 2026 at 10:49:59AM +0200, Romain Gantois wrote:
>
> > > > iio_multiply_value() passes integers val and val2 directly to abs(). This
> > > > is problematic because if a signed argument to abs is the lowest value for
> > > > its type, then the result is undefined due to overflow.
> > > >
> > > > Cast val and val2 to s64 before passing them to abs() to avoid this issue.
>
> ...
>
> > I've just looked at the 'work of art' that is abs().
> > What is wrong with:
> > #define abs(x) (sizeof(x) == sizeof(long long) ? __abs(long long, x) : \
> > __abs(int, x))
> > #define __abs(type, x) \
> > ({ type __abs_x = (x); __abs_x < 0 ? -__abs_x : __abs_x;})
> >
> > It is just as broken for u128.
> > It will use the correct signedness for char (but it is unsigned now).
> > It doesn't cast back to char, but that is entirely pointless unless code
> > looks at the type of the expression, the return value itself is always
> > promoted to int before being used.
> >
> > Actually replace the -__abs_x (UB for INT_MIN) with the safe:
> > (unsigned type)-(__abs_x + 1) + 1
> > and the return type will be unsigned with a correct value for -INT_MIN.
> > (Oh and the compiler sees through the mess.)
>
> And this is definitely wrong. We must keep type, because abs() might be used in
> the comparisons with signed or as parameter to multiplication or division where
> sign has to be preserved.
Thinks.... (bad at 11pm)
IIRC -INT_MIN is UB, but (~INT_MIN + 1) is fine provided -fno-strict-overflow
is set - which it is for kernel builds.
At least that guarantees the abs(-INT_MIN) == INT_MIN which is about the best
you can do.
It isn't as if it is ever going to happen.
There are all sorts of ways to break things in a driver.
David
prev parent reply other threads:[~2026-03-31 22:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-31 8:49 [PATCH] iio: inkern: Avoid risky abs() usage in iio_multiply_value() Romain Gantois
2026-03-31 9:29 ` Andy Shevchenko
2026-03-31 9:30 ` Andy Shevchenko
2026-03-31 10:08 ` Andy Shevchenko
2026-03-31 12:13 ` Romain Gantois
2026-03-31 18:37 ` Andy Shevchenko
2026-03-31 15:26 ` David Laight
2026-03-31 18:34 ` Andy Shevchenko
2026-03-31 22:04 ` David Laight [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260331230451.6c0bd155@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=hansg@kernel.org \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
--cc=romain.gantois@bootlin.com \
--cc=stable@vger.kernel.org \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.