From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add iio_str_to_fixpoint64() function that leverages simple_strtoull()
to parse numbers from a string.
A helper function __iio_str_to_fixpoint64() replaces
__iio_str_to_fixpoint() implementation, extending its usage for
64-bit fixed-point parsing.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/industrialio-core.c | 211 ++++++++++++++++++++++++++++++----------
include/linux/iio/iio.h | 2 +
2 files changed, 163 insertions(+), 50 deletions(-)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 3115d59c1372..37e9ed6b659b 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -21,6 +21,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/overflow.h>
#include <linux/poll.h>
#include <linux/property.h>
#include <linux/sched.h>
@@ -881,6 +882,136 @@ static ssize_t iio_read_channel_info_avail(struct device *dev,
}
}
+/**
+ * iio_safe_strntou64() - Parse u64 from string checking for overflow safety
+ * @str: The string to parse
+ * @endp: output pointer to the end parsing position
+ * @result: parsed value
+ * @max_chars: maximum number of digit characters to read
+ *
+ * This function is used in fixed-point parsing and it iterates over a const
+ * char array. It might duplicate behavior of simple_strtoull() or kstrtoull(),
+ * but those have their own limitations:
+ * - simple_strtoull() is not overflow-safe and its usage is discouraged;
+ * - kstrtoull() is safe, but requires termination and it would required a copy
+ * of the string to a temporary buffer.
+ *
+ * The implementation of this function is similar to _parse_integer_limit()
+ * available in lib/kstrtox.h, but that header/function is not available to be
+ * used in kernel modules. Hence, this implementation may need to change or
+ * removed to reuse a new suitable helper that is properly exposed.
+ *
+ * Returns:
+ * number of parsed characters on success, -ERANGE on overflow
+ */
+static ssize_t iio_safe_strntou64(const char *str, const char **endp,
+ u64 *result, size_t max_chars)
+{
+ u64 digit, acc = 0;
+ ssize_t idx = 0;
+
+ while (isdigit(str[idx]) && idx < max_chars) {
+ digit = str[idx] - '0';
+ if (unlikely(acc & (~0ull << 60))) {
+ if (check_mul_overflow(acc, 10, &acc) ||
+ check_add_overflow(acc, digit, &acc))
+ return -ERANGE;
+ } else {
+ acc = acc * 10 + digit;
+ }
+ idx++;
+ }
+
+ *endp = str + idx;
+ *result = acc;
+ return idx;
+}
+
+/**
+ * __iio_str_to_fixpoint64() - Parse a fixed-point number from a string
+ * @str: The string to parse
+ * @fract_mult: Multiplier for the first decimal place, should be a power of 10
+ * @integer: The integer part of the number
+ * @fract: The fractional part of the number
+ * @scale_db: True if this should parse as dB
+ *
+ * This variant uses 64-bit integers for both integer and fractional parts.
+ * Parsed positive values greater than S64_MAX are returned as-is. Parsed
+ * negative values less than S64_MIN are treated as range error, so -ERANGE is
+ * returned.
+ *
+ * Returns:
+ * 0 on success, or a negative error code if the string could not be parsed.
+ */
+static int __iio_str_to_fixpoint64(const char *str, u64 fract_mult,
+ s64 *integer, s64 *fract, bool scale_db)
+{
+ u64 i = 0, f = 0;
+ int ret, precision = ffs(fract_mult);
+ bool negative = false;
+
+ if (precision > 20) /* ceil(log10(U64_MAX)) = 20 */
+ return -EINVAL;
+
+ if (str[0] == '-') {
+ negative = true;
+ str++;
+ } else if (str[0] == '+') {
+ str++;
+ }
+
+ ret = iio_safe_strntou64(str, &str, &i, SIZE_MAX);
+ if (ret < 0)
+ return ret;
+
+ if (precision && *str == '.') {
+ str++; /* skip decimal point */
+ ret = iio_safe_strntou64(str, &str, &f, precision);
+ if (ret < 0)
+ return ret;
+
+ if (ret < precision) /* scale up */
+ f *= int_pow(10, precision - ret);
+
+ while (isdigit(*str)) /* truncate: ignore further digits */
+ str++;
+ }
+
+ if (!ret)
+ return -EINVAL;
+
+ if (scale_db) {
+ /* Ignore the dB suffix */
+ if (!strncmp(str, " dB", sizeof(" dB") - 1))
+ str += sizeof(" dB") - 1;
+ else if (!strncmp(str, "dB", sizeof("dB") - 1))
+ str += sizeof("dB") - 1;
+ }
+
+ if (*str == '\n')
+ str++;
+
+ if (*str != '\0')
+ return -EINVAL;
+
+ if (negative) {
+ if (i) {
+ if (i > (u64)S64_MIN)
+ return -ERANGE;
+ i = -i;
+ } else {
+ if (f > (u64)S64_MIN)
+ return -ERANGE;
+ f = -f;
+ }
+ }
+
+ *integer = i;
+ *fract = f;
+
+ return 0;
+}
+
/**
* __iio_str_to_fixpoint() - Parse a fixed-point number from a string
* @str: The string to parse
@@ -895,63 +1026,43 @@ static ssize_t iio_read_channel_info_avail(struct device *dev,
static int __iio_str_to_fixpoint(const char *str, int fract_mult,
int *integer, int *fract, bool scale_db)
{
- int i = 0, f = 0;
- bool integer_part = true, negative = false;
+ s64 integer64, fract64;
+ int ret;
- if (fract_mult == 0) {
- *fract = 0;
+ ret = __iio_str_to_fixpoint64(str, fract_mult, &integer64, &fract64,
+ scale_db);
+ if (ret)
+ return ret;
- return kstrtoint(str, 0, integer);
- }
+ if (integer64 < INT_MIN || integer64 > UINT_MAX ||
+ fract64 < INT_MIN || fract64 > UINT_MAX)
+ return -ERANGE;
- if (str[0] == '-') {
- negative = true;
- str++;
- } else if (str[0] == '+') {
- str++;
- }
-
- while (*str) {
- if ('0' <= *str && *str <= '9') {
- if (integer_part) {
- i = i * 10 + *str - '0';
- } else {
- f += fract_mult * (*str - '0');
- fract_mult /= 10;
- }
- } else if (*str == '\n') {
- if (*(str + 1) == '\0')
- break;
- return -EINVAL;
- } else if (!strncmp(str, " dB", sizeof(" dB") - 1) && scale_db) {
- /* Ignore the dB suffix */
- str += sizeof(" dB") - 1;
- continue;
- } else if (!strncmp(str, "dB", sizeof("dB") - 1) && scale_db) {
- /* Ignore the dB suffix */
- str += sizeof("dB") - 1;
- continue;
- } else if (*str == '.' && integer_part) {
- integer_part = false;
- } else {
- return -EINVAL;
- }
- str++;
- }
-
- if (negative) {
- if (i)
- i = -i;
- else
- f = -f;
- }
-
- *integer = i;
- *fract = f;
+ *integer = integer64;
+ *fract = fract64;
return 0;
}
+/**
+ * iio_str_to_fixpoint64() - Parse a fixed-point number from a string
+ * @str: The string to parse
+ * @fract_mult: Multiplier for the first decimal place, should be a power of 10
+ * @integer: The integer part of the number
+ * @fract: The fractional part of the number
+ *
+ * This variant uses 64-bit integers for both integer and fractional parts.
+ *
+ * Returns:
+ * 0 on success, or a negative error code if the string could not be parsed.
+ */
+int iio_str_to_fixpoint64(const char *str, u64 fract_mult, s64 *integer,
+ s64 *fract)
+{
+ return __iio_str_to_fixpoint64(str, fract_mult, integer, fract, false);
+}
+EXPORT_SYMBOL_GPL(iio_str_to_fixpoint64);
+
/**
* iio_str_to_fixpoint() - Parse a fixed-point number from a string
* @str: The string to parse
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index a9ecff191bd9..cb30d153465a 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -1055,6 +1055,8 @@ int iio_active_scan_mask_index(struct iio_dev *indio_dev);
ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals);
+int iio_str_to_fixpoint64(const char *str, u64 fract_mult, s64 *integer,
+ s64 *fract);
int iio_str_to_fixpoint(const char *str, int fract_mult, int *integer,
int *fract);
--
2.43.0On Mon, 16 Feb 2026 15:02:17 +0000 Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote: > From: Rodrigo Alencar <rodrigo.alencar@analog.com> > > Add iio_str_to_fixpoint64() function that leverages simple_strtoull() > to parse numbers from a string. > A helper function __iio_str_to_fixpoint64() replaces > __iio_str_to_fixpoint() implementation, extending its usage for > 64-bit fixed-point parsing. > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> Hi Rodrigo. This looks fine to me, but given earlier discussions I'd ideally like to wait for a final review from Andy. Thanks, Jonathan
On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote: > On Mon, 16 Feb 2026 15:02:17 +0000 > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote: > > > Add iio_str_to_fixpoint64() function that leverages simple_strtoull() > > to parse numbers from a string. > > A helper function __iio_str_to_fixpoint64() replaces > > __iio_str_to_fixpoint() implementation, extending its usage for > > 64-bit fixed-point parsing. > > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> > Hi Rodrigo. > > This looks fine to me, but given earlier discussions I'd ideally like > to wait for a final review from Andy. It all depends on the series from Dmitry Antipov. Can somebody help reviewing the patch 1 there? https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/ When it's in, we can continue on this one. TL;DR: for me this is on hold. But if you see the need to have the driver being in IIO, please add a big fat FIXME to make sure we will get this all being sorted out in the (nearest?) future. -- With Best Regards, Andy Shevchenko
On 26/02/23 11:06AM, Andy Shevchenko wrote: > On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote: > > On Mon, 16 Feb 2026 15:02:17 +0000 > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote: > > > > > Add iio_str_to_fixpoint64() function that leverages simple_strtoull() > > > to parse numbers from a string. > > > A helper function __iio_str_to_fixpoint64() replaces > > > __iio_str_to_fixpoint() implementation, extending its usage for > > > 64-bit fixed-point parsing. > > > > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> > > Hi Rodrigo. > > > > This looks fine to me, but given earlier discussions I'd ideally like > > to wait for a final review from Andy. > > It all depends on the series from Dmitry Antipov. > Can somebody help reviewing the patch 1 there? > https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/ can we push for the exposure of that function to kernel modules? We have discussed that in v6, and I understand that: EXPORT_SYMBOL_FOR_MODULES(_parse_integer_limit, "industrialio"); in lib/kstrtox.c; #include "../../lib/kstrtox.h" in drivers/iio/industrialio-core.c is not a good call... > When it's in, we can continue on this one. TL;DR: for me this is on hold. > But if you see the need to have the driver being in IIO, please add a big > fat FIXME to make sure we will get this all being sorted out in the > (nearest?) future. I could add the FIXME into iio_safe_strntou64() doc header. It explains the context: > + * The implementation of this function is similar to _parse_integer_limit() > + * available in lib/kstrtox.h, but that header/function is not available to be > + * used in kernel modules. Hence, this implementation may need to change or > + * removed to reuse a new suitable helper that is properly exposed. -- Kind regards, Rodrigo Alencar
On Mon, Feb 23, 2026 at 12:37 PM Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote: > On 26/02/23 11:06AM, Andy Shevchenko wrote: > > On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote: > > > On Mon, 16 Feb 2026 15:02:17 +0000 > > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote: > > > > > > > Add iio_str_to_fixpoint64() function that leverages simple_strtoull() > > > > to parse numbers from a string. > > > > A helper function __iio_str_to_fixpoint64() replaces > > > > __iio_str_to_fixpoint() implementation, extending its usage for > > > > 64-bit fixed-point parsing. > > > > > > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> > > > Hi Rodrigo. > > > > > > This looks fine to me, but given earlier discussions I'd ideally like > > > to wait for a final review from Andy. > > > > It all depends on the series from Dmitry Antipov. > > Can somebody help reviewing the patch 1 there? > > https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/ > > can we push for the exposure of that function to kernel modules? > We have discussed that in v6, and I understand that: > > EXPORT_SYMBOL_FOR_MODULES(_parse_integer_limit, "industrialio"); > in lib/kstrtox.c; > > #include "../../lib/kstrtox.h" > in drivers/iio/industrialio-core.c > > is not a good call... Yep, because it's a temporary band-aid. The proper solution is to have shared code provided by the lib/. So, the wrapper to parse 64-bit out from the constant string literal should be part of the lib/ in the result. > > When it's in, we can continue on this one. TL;DR: for me this is on hold. > > But if you see the need to have the driver being in IIO, please add a big > > fat FIXME to make sure we will get this all being sorted out in the > > (nearest?) future. > > I could add the FIXME into iio_safe_strntou64() doc header. It explains > the context: > > > + * The implementation of this function is similar to _parse_integer_limit() > > + * available in lib/kstrtox.h, but that header/function is not available to be > > + * used in kernel modules. Hence, this implementation may need to change or > > + * removed to reuse a new suitable helper that is properly exposed. Up to Jonathan, I hope we can move the above mentioned series forward. Without that, as I pointed out, this one sounds to me suboptimal and unneeded double effort. -- With Best Regards, Andy Shevchenko
On Mon, 23 Feb 2026 12:41:45 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Feb 23, 2026 at 12:37 PM Rodrigo Alencar > <455.rodrigo.alencar@gmail.com> wrote: > > On 26/02/23 11:06AM, Andy Shevchenko wrote: > > > On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote: > > > > On Mon, 16 Feb 2026 15:02:17 +0000 > > > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote: > > > > > > > > > Add iio_str_to_fixpoint64() function that leverages simple_strtoull() > > > > > to parse numbers from a string. > > > > > A helper function __iio_str_to_fixpoint64() replaces > > > > > __iio_str_to_fixpoint() implementation, extending its usage for > > > > > 64-bit fixed-point parsing. > > > > > > > > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com> > > > > Hi Rodrigo. > > > > > > > > This looks fine to me, but given earlier discussions I'd ideally like > > > > to wait for a final review from Andy. > > > > > > It all depends on the series from Dmitry Antipov. > > > Can somebody help reviewing the patch 1 there? > > > https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/ > > > > can we push for the exposure of that function to kernel modules? > > We have discussed that in v6, and I understand that: > > > > EXPORT_SYMBOL_FOR_MODULES(_parse_integer_limit, "industrialio"); > > in lib/kstrtox.c; > > > > #include "../../lib/kstrtox.h" > > in drivers/iio/industrialio-core.c > > > > is not a good call... > > Yep, because it's a temporary band-aid. The proper solution is to have > shared code provided by the lib/. So, the wrapper to parse 64-bit out > from the constant string literal should be part of the lib/ in the > result. > > > > When it's in, we can continue on this one. TL;DR: for me this is on hold. > > > But if you see the need to have the driver being in IIO, please add a big > > > fat FIXME to make sure we will get this all being sorted out in the > > > (nearest?) future. > > > > I could add the FIXME into iio_safe_strntou64() doc header. It explains > > the context: > > > > > + * The implementation of this function is similar to _parse_integer_limit() > > > + * available in lib/kstrtox.h, but that header/function is not available to be > > > + * used in kernel modules. Hence, this implementation may need to change or > > > + * removed to reuse a new suitable helper that is properly exposed. > > Up to Jonathan, I hope we can move the above mentioned series forward. > Without that, as I pointed out, this one sounds to me suboptimal and > unneeded double effort. > I don't want to hold this series for another cycle, but we are still fairly early in this one, so some focus on moving that forwards seems sensible. If we are running out of time, we can fallback to a loud FIXME and a plan to move to the generic version in the library next cycle. So let's set a rough deadline of rc5 and see how things are going then. For now I'm going to mark this series as "changes requested" in patchwork and stop tracking it. Jonathan >
On Sun, Mar 01, 2026 at 12:23:40PM +0000, Jonathan Cameron wrote: > On Mon, 23 Feb 2026 12:41:45 +0200 > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Feb 23, 2026 at 12:37 PM Rodrigo Alencar > > <455.rodrigo.alencar@gmail.com> wrote: > > > On 26/02/23 11:06AM, Andy Shevchenko wrote: > > > > On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote: ... > > > > It all depends on the series from Dmitry Antipov. > > > > Can somebody help reviewing the patch 1 there? > > > > https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/ FWIW, Andrew picked them up for Linux Next. Please, test! > > > can we push for the exposure of that function to kernel modules? > > > We have discussed that in v6, and I understand that: > > > > > > EXPORT_SYMBOL_FOR_MODULES(_parse_integer_limit, "industrialio"); > > > in lib/kstrtox.c; > > > > > > #include "../../lib/kstrtox.h" > > > in drivers/iio/industrialio-core.c > > > > > > is not a good call... > > > > Yep, because it's a temporary band-aid. The proper solution is to have > > shared code provided by the lib/. So, the wrapper to parse 64-bit out > > from the constant string literal should be part of the lib/ in the > > result. > > > > > > When it's in, we can continue on this one. TL;DR: for me this is on hold. > > > > But if you see the need to have the driver being in IIO, please add a big > > > > fat FIXME to make sure we will get this all being sorted out in the > > > > (nearest?) future. > > > > > > I could add the FIXME into iio_safe_strntou64() doc header. It explains > > > the context: > > > > > > > + * The implementation of this function is similar to _parse_integer_limit() > > > > + * available in lib/kstrtox.h, but that header/function is not available to be > > > > + * used in kernel modules. Hence, this implementation may need to change or > > > > + * removed to reuse a new suitable helper that is properly exposed. > > > > Up to Jonathan, I hope we can move the above mentioned series forward. > > Without that, as I pointed out, this one sounds to me suboptimal and > > unneeded double effort. > > > I don't want to hold this series for another cycle, but we are still > fairly early in this one, so some focus on moving that forwards seems > sensible. If we are running out of time, we can fallback to a loud > FIXME and a plan to move to the generic version in the library next cycle. > So let's set a rough deadline of rc5 and see how things are going then. Taking into account the above, can we actually develop something based on that? Or at least having a temporary solution for this cycle followed up by the better one for the next? -- With Best Regards, Andy Shevchenko
On 26/03/02 10:27AM, Andy Shevchenko wrote: > On Sun, Mar 01, 2026 at 12:23:40PM +0000, Jonathan Cameron wrote: > > On Mon, 23 Feb 2026 12:41:45 +0200 > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > On Mon, Feb 23, 2026 at 12:37 PM Rodrigo Alencar > > > <455.rodrigo.alencar@gmail.com> wrote: > > > > On 26/02/23 11:06AM, Andy Shevchenko wrote: > > > > > On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote: > > ... > > > > > > It all depends on the series from Dmitry Antipov. > > > > > Can somebody help reviewing the patch 1 there? > > > > > https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/ > > FWIW, Andrew picked them up for Linux Next. Please, test! The patch looks ok, but I am not seeing it solving my problem here. Here is the v8: https://lore.kernel.org/linux-hardening/aZXDSbyH8tWmTPPL@smile.fi.intel.com/T/#t I would have to use simple_strtoull() and it would clamp the value at ULLONG_MAX in case of overflow, but it would not say that an overflow happened. Would that be fine? I understand that addressing the FIXME in simple_strntoull() is not a subject of this patch. > > > > can we push for the exposure of that function to kernel modules? > > > > We have discussed that in v6, and I understand that: > > > > > > > > EXPORT_SYMBOL_FOR_MODULES(_parse_integer_limit, "industrialio"); > > > > in lib/kstrtox.c; > > > > > > > > #include "../../lib/kstrtox.h" > > > > in drivers/iio/industrialio-core.c > > > > > > > > is not a good call... > > > > > > Yep, because it's a temporary band-aid. The proper solution is to have > > > shared code provided by the lib/. So, the wrapper to parse 64-bit out > > > from the constant string literal should be part of the lib/ in the > > > result. > > > > > > > > When it's in, we can continue on this one. TL;DR: for me this is on hold. > > > > > But if you see the need to have the driver being in IIO, please add a big > > > > > fat FIXME to make sure we will get this all being sorted out in the > > > > > (nearest?) future. > > > > > > > > I could add the FIXME into iio_safe_strntou64() doc header. It explains > > > > the context: > > > > > > > > > + * The implementation of this function is similar to _parse_integer_limit() > > > > > + * available in lib/kstrtox.h, but that header/function is not available to be > > > > > + * used in kernel modules. Hence, this implementation may need to change or > > > > > + * removed to reuse a new suitable helper that is properly exposed. > > > > > > Up to Jonathan, I hope we can move the above mentioned series forward. > > > Without that, as I pointed out, this one sounds to me suboptimal and > > > unneeded double effort. > > > > > I don't want to hold this series for another cycle, but we are still > > fairly early in this one, so some focus on moving that forwards seems > > sensible. If we are running out of time, we can fallback to a loud > > FIXME and a plan to move to the generic version in the library next cycle. > > So let's set a rough deadline of rc5 and see how things are going then. > > Taking into account the above, can we actually develop something > based on that? Or at least having a temporary solution for this > cycle followed up by the better one for the next? As mentioned above, I am not sure how consume what Andrew has over there. It seems address lib/ internal stuff. The interfaces are still the same. -- Kind regards, Rodrigo Alencar
On Mon, Mar 02, 2026 at 09:19:42AM +0000, Rodrigo Alencar wrote:
> On 26/03/02 10:27AM, Andy Shevchenko wrote:
> > On Sun, Mar 01, 2026 at 12:23:40PM +0000, Jonathan Cameron wrote:
> > > On Mon, 23 Feb 2026 12:41:45 +0200
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > On Mon, Feb 23, 2026 at 12:37 PM Rodrigo Alencar
> > > > <455.rodrigo.alencar@gmail.com> wrote:
> > > > > On 26/02/23 11:06AM, Andy Shevchenko wrote:
> > > > > > On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote:
...
> > > > > > It all depends on the series from Dmitry Antipov.
> > > > > > Can somebody help reviewing the patch 1 there?
> > > > > > https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/
> >
> > FWIW, Andrew picked them up for Linux Next. Please, test!
>
> The patch looks ok, but I am not seeing it solving my problem here.
> Here is the v8:
> https://lore.kernel.org/linux-hardening/aZXDSbyH8tWmTPPL@smile.fi.intel.com/T/#t
>
> I would have to use simple_strtoull() and it would clamp the value at
> ULLONG_MAX in case of overflow, but it would not say that an overflow
> happened. Would that be fine? I understand that addressing the FIXME
> in simple_strntoull() is not a subject of this patch.
>
> > > > > can we push for the exposure of that function to kernel modules?
> > > > > We have discussed that in v6, and I understand that:
> > > > >
> > > > > EXPORT_SYMBOL_FOR_MODULES(_parse_integer_limit, "industrialio");
> > > > > in lib/kstrtox.c;
> > > > >
> > > > > #include "../../lib/kstrtox.h"
> > > > > in drivers/iio/industrialio-core.c
> > > > >
> > > > > is not a good call...
> > > >
> > > > Yep, because it's a temporary band-aid. The proper solution is to have
> > > > shared code provided by the lib/. So, the wrapper to parse 64-bit out
> > > > from the constant string literal should be part of the lib/ in the
> > > > result.
> > > >
> > > > > > When it's in, we can continue on this one. TL;DR: for me this is on hold.
> > > > > > But if you see the need to have the driver being in IIO, please add a big
> > > > > > fat FIXME to make sure we will get this all being sorted out in the
> > > > > > (nearest?) future.
> > > > >
> > > > > I could add the FIXME into iio_safe_strntou64() doc header. It explains
> > > > > the context:
> > > > >
> > > > > > + * The implementation of this function is similar to _parse_integer_limit()
> > > > > > + * available in lib/kstrtox.h, but that header/function is not available to be
> > > > > > + * used in kernel modules. Hence, this implementation may need to change or
> > > > > > + * removed to reuse a new suitable helper that is properly exposed.
> > > >
> > > > Up to Jonathan, I hope we can move the above mentioned series forward.
> > > > Without that, as I pointed out, this one sounds to me suboptimal and
> > > > unneeded double effort.
> > > >
> > > I don't want to hold this series for another cycle, but we are still
> > > fairly early in this one, so some focus on moving that forwards seems
> > > sensible. If we are running out of time, we can fallback to a loud
> > > FIXME and a plan to move to the generic version in the library next cycle.
> > > So let's set a rough deadline of rc5 and see how things are going then.
> >
> > Taking into account the above, can we actually develop something
> > based on that? Or at least having a temporary solution for this
> > cycle followed up by the better one for the next?
>
> As mentioned above, I am not sure how consume what Andrew has over there.
> It seems address lib/ internal stuff. The interfaces are still the same.
I think it will be third time I'm repeating that this needs a wrapper in the
lib/. Just add the one (like with safe_strtoull() naming schema) with properly
formed prototype that returns an error and the result in different variables
int safe_strtoull(..., *result)
{
...
}
EXPORT_SYMBOL_GPL(safe_strtoull);
(Also some test cases have to be added.)
--
With Best Regards,
Andy Shevchenko
On 26/03/02 11:33AM, Andy Shevchenko wrote:
> On Mon, Mar 02, 2026 at 09:19:42AM +0000, Rodrigo Alencar wrote:
> > On 26/03/02 10:27AM, Andy Shevchenko wrote:
> > > On Sun, Mar 01, 2026 at 12:23:40PM +0000, Jonathan Cameron wrote:
> > > > On Mon, 23 Feb 2026 12:41:45 +0200
> > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > On Mon, Feb 23, 2026 at 12:37 PM Rodrigo Alencar
> > > > > <455.rodrigo.alencar@gmail.com> wrote:
> > > > > > On 26/02/23 11:06AM, Andy Shevchenko wrote:
> > > > > > > On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote:
>
> ...
>
> > > > > > > It all depends on the series from Dmitry Antipov.
> > > > > > > Can somebody help reviewing the patch 1 there?
> > > > > > > https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/
> > >
> > > FWIW, Andrew picked them up for Linux Next. Please, test!
> >
> > The patch looks ok, but I am not seeing it solving my problem here.
> > Here is the v8:
> > https://lore.kernel.org/linux-hardening/aZXDSbyH8tWmTPPL@smile.fi.intel.com/T/#t
> >
> > I would have to use simple_strtoull() and it would clamp the value at
> > ULLONG_MAX in case of overflow, but it would not say that an overflow
> > happened. Would that be fine? I understand that addressing the FIXME
> > in simple_strntoull() is not a subject of this patch.
> >
> > > > > > can we push for the exposure of that function to kernel modules?
> > > > > > We have discussed that in v6, and I understand that:
> > > > > >
> > > > > > EXPORT_SYMBOL_FOR_MODULES(_parse_integer_limit, "industrialio");
> > > > > > in lib/kstrtox.c;
> > > > > >
> > > > > > #include "../../lib/kstrtox.h"
> > > > > > in drivers/iio/industrialio-core.c
> > > > > >
> > > > > > is not a good call...
> > > > >
> > > > > Yep, because it's a temporary band-aid. The proper solution is to have
> > > > > shared code provided by the lib/. So, the wrapper to parse 64-bit out
> > > > > from the constant string literal should be part of the lib/ in the
> > > > > result.
> > > > >
> > > > > > > When it's in, we can continue on this one. TL;DR: for me this is on hold.
> > > > > > > But if you see the need to have the driver being in IIO, please add a big
> > > > > > > fat FIXME to make sure we will get this all being sorted out in the
> > > > > > > (nearest?) future.
> > > > > >
> > > > > > I could add the FIXME into iio_safe_strntou64() doc header. It explains
> > > > > > the context:
> > > > > >
> > > > > > > + * The implementation of this function is similar to _parse_integer_limit()
> > > > > > > + * available in lib/kstrtox.h, but that header/function is not available to be
> > > > > > > + * used in kernel modules. Hence, this implementation may need to change or
> > > > > > > + * removed to reuse a new suitable helper that is properly exposed.
> > > > >
> > > > > Up to Jonathan, I hope we can move the above mentioned series forward.
> > > > > Without that, as I pointed out, this one sounds to me suboptimal and
> > > > > unneeded double effort.
> > > > >
> > > > I don't want to hold this series for another cycle, but we are still
> > > > fairly early in this one, so some focus on moving that forwards seems
> > > > sensible. If we are running out of time, we can fallback to a loud
> > > > FIXME and a plan to move to the generic version in the library next cycle.
> > > > So let's set a rough deadline of rc5 and see how things are going then.
> > >
> > > Taking into account the above, can we actually develop something
> > > based on that? Or at least having a temporary solution for this
> > > cycle followed up by the better one for the next?
> >
> > As mentioned above, I am not sure how consume what Andrew has over there.
> > It seems address lib/ internal stuff. The interfaces are still the same.
>
> I think it will be third time I'm repeating that this needs a wrapper in the
> lib/. Just add the one (like with safe_strtoull() naming schema) with properly
> formed prototype that returns an error and the result in different variables
>
> int safe_strtoull(..., *result)
> {
> ...
> }
> EXPORT_SYMBOL_GPL(safe_strtoull);
>
> (Also some test cases have to be added.)
I really understood that I would need to use Andrew's work as is, but
in fact, you encouraging me to add what we need on top of it.
Assuming that this would increase the scope of this patch series, I suppose
that would need to be included in a separate one.
can we just not export simple_strntoull() from lib/vsprintf.c? I mean,
addressing its FIXME and changing its prototype?
--
Kind regards,
Rodrigo Alencar
On Mon, Mar 02, 2026 at 10:13:52AM +0000, Rodrigo Alencar wrote:
> On 26/03/02 11:33AM, Andy Shevchenko wrote:
> > On Mon, Mar 02, 2026 at 09:19:42AM +0000, Rodrigo Alencar wrote:
> > > On 26/03/02 10:27AM, Andy Shevchenko wrote:
> > > > On Sun, Mar 01, 2026 at 12:23:40PM +0000, Jonathan Cameron wrote:
> > > > > On Mon, 23 Feb 2026 12:41:45 +0200
> > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > On Mon, Feb 23, 2026 at 12:37 PM Rodrigo Alencar
> > > > > > <455.rodrigo.alencar@gmail.com> wrote:
> > > > > > > On 26/02/23 11:06AM, Andy Shevchenko wrote:
> > > > > > > > On Sun, Feb 22, 2026 at 05:29:12PM +0000, Jonathan Cameron wrote:
...
> > > > > > > > It all depends on the series from Dmitry Antipov.
> > > > > > > > Can somebody help reviewing the patch 1 there?
> > > > > > > > https://lore.kernel.org/linux-hardening/20260212125628.739276-1-dmantipov@yandex.ru/
> > > >
> > > > FWIW, Andrew picked them up for Linux Next. Please, test!
> > >
> > > The patch looks ok, but I am not seeing it solving my problem here.
> > > Here is the v8:
> > > https://lore.kernel.org/linux-hardening/aZXDSbyH8tWmTPPL@smile.fi.intel.com/T/#t
> > >
> > > I would have to use simple_strtoull() and it would clamp the value at
> > > ULLONG_MAX in case of overflow, but it would not say that an overflow
> > > happened. Would that be fine? I understand that addressing the FIXME
> > > in simple_strntoull() is not a subject of this patch.
> > >
> > > > > > > can we push for the exposure of that function to kernel modules?
> > > > > > > We have discussed that in v6, and I understand that:
> > > > > > >
> > > > > > > EXPORT_SYMBOL_FOR_MODULES(_parse_integer_limit, "industrialio");
> > > > > > > in lib/kstrtox.c;
> > > > > > >
> > > > > > > #include "../../lib/kstrtox.h"
> > > > > > > in drivers/iio/industrialio-core.c
> > > > > > >
> > > > > > > is not a good call...
> > > > > >
> > > > > > Yep, because it's a temporary band-aid. The proper solution is to have
> > > > > > shared code provided by the lib/. So, the wrapper to parse 64-bit out
> > > > > > from the constant string literal should be part of the lib/ in the
> > > > > > result.
> > > > > >
> > > > > > > > When it's in, we can continue on this one. TL;DR: for me this is on hold.
> > > > > > > > But if you see the need to have the driver being in IIO, please add a big
> > > > > > > > fat FIXME to make sure we will get this all being sorted out in the
> > > > > > > > (nearest?) future.
> > > > > > >
> > > > > > > I could add the FIXME into iio_safe_strntou64() doc header. It explains
> > > > > > > the context:
> > > > > > >
> > > > > > > > + * The implementation of this function is similar to _parse_integer_limit()
> > > > > > > > + * available in lib/kstrtox.h, but that header/function is not available to be
> > > > > > > > + * used in kernel modules. Hence, this implementation may need to change or
> > > > > > > > + * removed to reuse a new suitable helper that is properly exposed.
> > > > > >
> > > > > > Up to Jonathan, I hope we can move the above mentioned series forward.
> > > > > > Without that, as I pointed out, this one sounds to me suboptimal and
> > > > > > unneeded double effort.
> > > > > >
> > > > > I don't want to hold this series for another cycle, but we are still
> > > > > fairly early in this one, so some focus on moving that forwards seems
> > > > > sensible. If we are running out of time, we can fallback to a loud
> > > > > FIXME and a plan to move to the generic version in the library next cycle.
> > > > > So let's set a rough deadline of rc5 and see how things are going then.
> > > >
> > > > Taking into account the above, can we actually develop something
> > > > based on that? Or at least having a temporary solution for this
> > > > cycle followed up by the better one for the next?
> > >
> > > As mentioned above, I am not sure how consume what Andrew has over there.
> > > It seems address lib/ internal stuff. The interfaces are still the same.
> >
> > I think it will be third time I'm repeating that this needs a wrapper in the
> > lib/. Just add the one (like with safe_strtoull() naming schema) with properly
> > formed prototype that returns an error and the result in different variables
> >
> > int safe_strtoull(..., *result)
> > {
> > ...
> > }
> > EXPORT_SYMBOL_GPL(safe_strtoull);
> >
> > (Also some test cases have to be added.)
>
> I really understood that I would need to use Andrew's work as is, but
> in fact, you encouraging me to add what we need on top of it.
>
> Assuming that this would increase the scope of this patch series, I suppose
> that would need to be included in a separate one.
> can we just not export simple_strntoull() from lib/vsprintf.c? I mean,
> addressing its FIXME and changing its prototype?
I do not understand why we have to stick with the scope of IIO and make an
unneeded churn and double effort. We have a code that you need. Yes, it
requires an additional change that adds a glue (exported) function. How is it
out of the scope? If we hurry up with that code to be added, it increases
the chances to get the rest in sooner.
--
With Best Regards,
Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.