[PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Amir Goldstein]

Code using ERR_PTR() is almost certainly intending to produce a value
which qualified as IS_ERR_OR_NULL(), but this is not the case when
code calls ERR_PTR(err) with positive or large negative err.

Introduce a fortified variant of ERR_PTR() whose return value is
guaranteed to qualify as IS_ERR_OR_NULL().

We add this in a new header file err_ptr.h which includes bug.h
for the build/run time assertions.

Subsystems may opt-in for fortified ERR_PTR() for specific call sites
or by #define ERR_PTR(err) ERR_PTR_SAFE(err).

Link: https://lore.kernel.org/r/CAOQ4uxg=gONUh5QEW5KJcyXLDF15HbLnc9Ea7RKPcgtyfPasTA@mail.gmail.com/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Guys,

Please follow the Link to see the sneaky bug that Nirmoy tracked down.
syzbot has complained about this a while ago, but neither me nor my AI
helpers were able to track it down from code analysis.

Honestly, with AI review, this class of bugs (return a stale err value)
should not be happening anymore, but it annoyed me that ERR_PTR() can
return a value which is not an IS_ERR(). It messes with code flow
analysis.

What do you think about this macro?

I intend to #define ERR_PTR(err) ERR_PTR_SAFE(err) in overlayfs.h
to fortify all of the ERR_PTR() in overlayfs code.

What do you think about this opt-in method?
Any reason to make this more widespread by default?

Thanks,
Amir.


 include/linux/err_ptr.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 include/linux/err_ptr.h

diff --git a/include/linux/err_ptr.h b/include/linux/err_ptr.h
new file mode 100644
index 0000000000000..829ec5f771528
--- /dev/null
+++ b/include/linux/err_ptr.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ERR_PTR_H
+#define _LINUX_ERR_PTR_H
+
+#include <linux/err.h>
+#include <linux/bug.h>
+
+/**
+ * ERR_PTR_SAFE - Create an error pointer, with validation.
+ * @error: An error code to encode as an error pointer.
+ *
+ * Like ERR_PTR(), but validates @error:
+ * - For constant @error: fails the build if the value is not a valid errno
+ *   (zero is allowed, producing NULL).
+ * - For variable @error: warns and clamps to -MAX_ERRNO if out of range.
+ *
+ * Subsystems may opt in for all ERR_PTR() call sites by adding after includes:
+ *   #undef ERR_PTR
+ *   #define ERR_PTR(err) ERR_PTR_SAFE(err)
+ */
+#define ERR_PTR_SAFE(error) ({						\
+	long __e = (error);						\
+	if (__builtin_constant_p(__e))					\
+		BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));		\
+	__builtin_constant_p(__e) ? (void *)__e :			\
+		(void *)(WARN_ON(__e && !IS_ERR_VALUE(__e)) ? -MAX_ERRNO : __e);\
+})
+
+#endif /* _LINUX_ERR_PTR_H */
-- 
2.54.0

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Nirmoy Das]

On 14.05.26 23:01, Amir Goldstein wrote:
> Code using ERR_PTR() is almost certainly intending to produce a value
> which qualified as IS_ERR_OR_NULL(), but this is not the case when
> code calls ERR_PTR(err) with positive or large negative err.
>
> Introduce a fortified variant of ERR_PTR() whose return value is
> guaranteed to qualify as IS_ERR_OR_NULL().
>
> We add this in a new header file err_ptr.h which includes bug.h
> for the build/run time assertions.
>
> Subsystems may opt-in for fortified ERR_PTR() for specific call sites
> or by #define ERR_PTR(err) ERR_PTR_SAFE(err).
>
> Link: https://lore.kernel.org/r/CAOQ4uxg=gONUh5QEW5KJcyXLDF15HbLnc9Ea7RKPcgtyfPasTA@mail.gmail.com/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

I tested this on top of Amir's ovl-fixes branch[0], with overlayfs opted 
in to
ERR_PTR_SAFE() and with ovl_iterate_merged() fix reverted.

The syz reproducer triggered the new WARN_ON() from ERR_PTR_SAFE():
WARNING: fs/overlayfs/readdir.c:511 at ovl_iterate+0x4c0/0x5bc
  Call trace:
    ovl_iterate+0x4c0/0x5bc
    wrap_directory_iterator+0x60/0x90
    shared_ovl_iterate+0x18/0x24
    iterate_dir+0x10c/0x3a4
    __arm64_sys_getdents64+0xe0/0x1e4


Tested-by: Nirmoy Das <nirmoyd@nvidia.com>
Acked-by: Nirmoy Das <nirmoyd@nvidia.com>

[0] https://github.com/amir73il/linux/commits/ovl-fixes/

> ---
>
> Guys,
>
> Please follow the Link to see the sneaky bug that Nirmoy tracked down.
> syzbot has complained about this a while ago, but neither me nor my AI
> helpers were able to track it down from code analysis.
>
> Honestly, with AI review, this class of bugs (return a stale err value)
> should not be happening anymore, but it annoyed me that ERR_PTR() can
> return a value which is not an IS_ERR(). It messes with code flow
> analysis.
>
> What do you think about this macro?
>
> I intend to #define ERR_PTR(err) ERR_PTR_SAFE(err) in overlayfs.h
> to fortify all of the ERR_PTR() in overlayfs code.
>
> What do you think about this opt-in method?
> Any reason to make this more widespread by default?
>
> Thanks,
> Amir.
>
>
>   include/linux/err_ptr.h | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
>   create mode 100644 include/linux/err_ptr.h
>
> diff --git a/include/linux/err_ptr.h b/include/linux/err_ptr.h
> new file mode 100644
> index 0000000000000..829ec5f771528
> --- /dev/null
> +++ b/include/linux/err_ptr.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_ERR_PTR_H
> +#define _LINUX_ERR_PTR_H
> +
> +#include <linux/err.h>
> +#include <linux/bug.h>
> +
> +/**
> + * ERR_PTR_SAFE - Create an error pointer, with validation.
> + * @error: An error code to encode as an error pointer.
> + *
> + * Like ERR_PTR(), but validates @error:
> + * - For constant @error: fails the build if the value is not a valid errno
> + *   (zero is allowed, producing NULL).
> + * - For variable @error: warns and clamps to -MAX_ERRNO if out of range.
> + *
> + * Subsystems may opt in for all ERR_PTR() call sites by adding after includes:
> + *   #undef ERR_PTR
> + *   #define ERR_PTR(err) ERR_PTR_SAFE(err)
> + */
> +#define ERR_PTR_SAFE(error) ({						\
> +	long __e = (error);						\
> +	if (__builtin_constant_p(__e))					\
> +		BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));		\
> +	__builtin_constant_p(__e) ? (void *)__e :			\
> +		(void *)(WARN_ON(__e && !IS_ERR_VALUE(__e)) ? -MAX_ERRNO : __e);\
> +})
> +
> +#endif /* _LINUX_ERR_PTR_H */

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Jori Koolstra]

On Thu, May 14, 2026 at 10:01:29PM +0200, Amir Goldstein wrote:
> Code using ERR_PTR() is almost certainly intending to produce a value
> which qualified as IS_ERR_OR_NULL(), but this is not the case when
> code calls ERR_PTR(err) with positive or large negative err.
> 
> Introduce a fortified variant of ERR_PTR() whose return value is
> guaranteed to qualify as IS_ERR_OR_NULL().
> 
> We add this in a new header file err_ptr.h which includes bug.h
> for the build/run time assertions.
> 
> Subsystems may opt-in for fortified ERR_PTR() for specific call sites
> or by #define ERR_PTR(err) ERR_PTR_SAFE(err).
> 
> Link: https://lore.kernel.org/r/CAOQ4uxg=gONUh5QEW5KJcyXLDF15HbLnc9Ea7RKPcgtyfPasTA@mail.gmail.com/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> 

This seems like a good idea. Perhaps, we should also not pass error
pointers as parameters. It just invites similar issues. For example,
in

	if (!error) {
		dentry = vfs_mkdir(mnt_idmap(path.mnt), path.dentry->d_inode,
				   dentry, mode, &delegated_inode);
		if (IS_ERR(dentry))
			error = PTR_ERR(dentry);
	}
	end_creating_path(&path, dentry);

Suppose you want to make changes to end_creating_path, nothing indicates
that dentry may hold an error here. In this case it is not too hard to
find out, but it seems like a bad idea in general. But I might miss
cases where this is really useful... :)


> Guys,
> 
> Please follow the Link to see the sneaky bug that Nirmoy tracked down.
> syzbot has complained about this a while ago, but neither me nor my AI
> helpers were able to track it down from code analysis.
> 
> Honestly, with AI review, this class of bugs (return a stale err value)
> should not be happening anymore, but it annoyed me that ERR_PTR() can
> return a value which is not an IS_ERR(). It messes with code flow
> analysis.
> 
> What do you think about this macro?
> 
> I intend to #define ERR_PTR(err) ERR_PTR_SAFE(err) in overlayfs.h
> to fortify all of the ERR_PTR() in overlayfs code.
> 
> What do you think about this opt-in method?
> Any reason to make this more widespread by default?
> 
> Thanks,
> Amir.
> 
> 
>  include/linux/err_ptr.h | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 include/linux/err_ptr.h
> 
> diff --git a/include/linux/err_ptr.h b/include/linux/err_ptr.h
> new file mode 100644
> index 0000000000000..829ec5f771528
> --- /dev/null
> +++ b/include/linux/err_ptr.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_ERR_PTR_H
> +#define _LINUX_ERR_PTR_H
> +
> +#include <linux/err.h>
> +#include <linux/bug.h>
> +
> +/**
> + * ERR_PTR_SAFE - Create an error pointer, with validation.
> + * @error: An error code to encode as an error pointer.
> + *
> + * Like ERR_PTR(), but validates @error:
> + * - For constant @error: fails the build if the value is not a valid errno
> + *   (zero is allowed, producing NULL).
> + * - For variable @error: warns and clamps to -MAX_ERRNO if out of range.
> + *
> + * Subsystems may opt in for all ERR_PTR() call sites by adding after includes:
> + *   #undef ERR_PTR
> + *   #define ERR_PTR(err) ERR_PTR_SAFE(err)
> + */
> +#define ERR_PTR_SAFE(error) ({						\
> +	long __e = (error);						\
> +	if (__builtin_constant_p(__e))					\
> +		BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));		\
> +	__builtin_constant_p(__e) ? (void *)__e :			\
> +		(void *)(WARN_ON(__e && !IS_ERR_VALUE(__e)) ? -MAX_ERRNO : __e);\
> +})
> +
> +#endif /* _LINUX_ERR_PTR_H */
> -- 
> 2.54.0
> 

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[David Laight]

On Thu, 14 May 2026 22:01:29 +0200
Amir Goldstein <amir73il@gmail.com> wrote:

> Code using ERR_PTR() is almost certainly intending to produce a value
> which qualified as IS_ERR_OR_NULL(), but this is not the case when
> code calls ERR_PTR(err) with positive or large negative err.
> 
> Introduce a fortified variant of ERR_PTR() whose return value is
> guaranteed to qualify as IS_ERR_OR_NULL().
> 
> We add this in a new header file err_ptr.h which includes bug.h
> for the build/run time assertions.
> 
> Subsystems may opt-in for fortified ERR_PTR() for specific call sites
> or by #define ERR_PTR(err) ERR_PTR_SAFE(err).
> 
> Link: https://lore.kernel.org/r/CAOQ4uxg=gONUh5QEW5KJcyXLDF15HbLnc9Ea7RKPcgtyfPasTA@mail.gmail.com/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> 
> Guys,
> 
> Please follow the Link to see the sneaky bug that Nirmoy tracked down.
> syzbot has complained about this a while ago, but neither me nor my AI
> helpers were able to track it down from code analysis.
> 
> Honestly, with AI review, this class of bugs (return a stale err value)
> should not be happening anymore, but it annoyed me that ERR_PTR() can
> return a value which is not an IS_ERR(). It messes with code flow
> analysis.
> 
> What do you think about this macro?
> 
> I intend to #define ERR_PTR(err) ERR_PTR_SAFE(err) in overlayfs.h
> to fortify all of the ERR_PTR() in overlayfs code.
> 
> What do you think about this opt-in method?
> Any reason to make this more widespread by default?
> 
> Thanks,
> Amir.
> 
> 
>  include/linux/err_ptr.h | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 include/linux/err_ptr.h
> 
> diff --git a/include/linux/err_ptr.h b/include/linux/err_ptr.h
> new file mode 100644
> index 0000000000000..829ec5f771528
> --- /dev/null
> +++ b/include/linux/err_ptr.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_ERR_PTR_H
> +#define _LINUX_ERR_PTR_H
> +
> +#include <linux/err.h>
> +#include <linux/bug.h>
> +
> +/**
> + * ERR_PTR_SAFE - Create an error pointer, with validation.
> + * @error: An error code to encode as an error pointer.
> + *
> + * Like ERR_PTR(), but validates @error:
> + * - For constant @error: fails the build if the value is not a valid errno
> + *   (zero is allowed, producing NULL).
> + * - For variable @error: warns and clamps to -MAX_ERRNO if out of range.
> + *
> + * Subsystems may opt in for all ERR_PTR() call sites by adding after includes:
> + *   #undef ERR_PTR
> + *   #define ERR_PTR(err) ERR_PTR_SAFE(err)
> + */
> +#define ERR_PTR_SAFE(error) ({						\
> +	long __e = (error);						\
> +	if (__builtin_constant_p(__e))					\
> +		BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));		\
> +	__builtin_constant_p(__e) ? (void *)__e :			\
> +		(void *)(WARN_ON(__e && !IS_ERR_VALUE(__e)) ? -MAX_ERRNO : __e);\

The object code bloat would be noticeable if this were used everywhere.
But you could make it a bit simpler:
	if (__builtin_constant_p(__e))
		BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
	else if WARN_ON(__e && !IS_ERR_VALUE(__e))
		__e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
	(void *)__e;
 
The check for constants may be fairly pointless.
One of the static checkers may already detect the obvious fubar ERR_PTR(EINVAL).

-- David

> +})
> +
> +#endif /* _LINUX_ERR_PTR_H */

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Amir Goldstein]

On Fri, May 15, 2026 at 8:30 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Thu, 14 May 2026 22:01:29 +0200
> Amir Goldstein <amir73il@gmail.com> wrote:
>
> > Code using ERR_PTR() is almost certainly intending to produce a value
> > which qualified as IS_ERR_OR_NULL(), but this is not the case when
> > code calls ERR_PTR(err) with positive or large negative err.
> >
> > Introduce a fortified variant of ERR_PTR() whose return value is
> > guaranteed to qualify as IS_ERR_OR_NULL().
> >
> > We add this in a new header file err_ptr.h which includes bug.h
> > for the build/run time assertions.
> >
> > Subsystems may opt-in for fortified ERR_PTR() for specific call sites
> > or by #define ERR_PTR(err) ERR_PTR_SAFE(err).
> >
> > Link: https://lore.kernel.org/r/CAOQ4uxg=gONUh5QEW5KJcyXLDF15HbLnc9Ea7RKPcgtyfPasTA@mail.gmail.com/
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Guys,
> >
> > Please follow the Link to see the sneaky bug that Nirmoy tracked down.
> > syzbot has complained about this a while ago, but neither me nor my AI
> > helpers were able to track it down from code analysis.
> >
> > Honestly, with AI review, this class of bugs (return a stale err value)
> > should not be happening anymore, but it annoyed me that ERR_PTR() can
> > return a value which is not an IS_ERR(). It messes with code flow
> > analysis.
> >
> > What do you think about this macro?
> >
> > I intend to #define ERR_PTR(err) ERR_PTR_SAFE(err) in overlayfs.h
> > to fortify all of the ERR_PTR() in overlayfs code.
> >
> > What do you think about this opt-in method?
> > Any reason to make this more widespread by default?
> >
> > Thanks,
> > Amir.
> >
> >
> >  include/linux/err_ptr.h | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >  create mode 100644 include/linux/err_ptr.h
> >
> > diff --git a/include/linux/err_ptr.h b/include/linux/err_ptr.h
> > new file mode 100644
> > index 0000000000000..829ec5f771528
> > --- /dev/null
> > +++ b/include/linux/err_ptr.h
> > @@ -0,0 +1,29 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _LINUX_ERR_PTR_H
> > +#define _LINUX_ERR_PTR_H
> > +
> > +#include <linux/err.h>
> > +#include <linux/bug.h>
> > +
> > +/**
> > + * ERR_PTR_SAFE - Create an error pointer, with validation.
> > + * @error: An error code to encode as an error pointer.
> > + *
> > + * Like ERR_PTR(), but validates @error:
> > + * - For constant @error: fails the build if the value is not a valid errno
> > + *   (zero is allowed, producing NULL).
> > + * - For variable @error: warns and clamps to -MAX_ERRNO if out of range.
> > + *
> > + * Subsystems may opt in for all ERR_PTR() call sites by adding after includes:
> > + *   #undef ERR_PTR
> > + *   #define ERR_PTR(err) ERR_PTR_SAFE(err)
> > + */
> > +#define ERR_PTR_SAFE(error) ({                                               \
> > +     long __e = (error);                                             \
> > +     if (__builtin_constant_p(__e))                                  \
> > +             BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));                \
> > +     __builtin_constant_p(__e) ? (void *)__e :                       \
> > +             (void *)(WARN_ON(__e && !IS_ERR_VALUE(__e)) ? -MAX_ERRNO : __e);\
>
> The object code bloat would be noticeable if this were used everywhere.
> But you could make it a bit simpler:
>         if (__builtin_constant_p(__e))
>                 BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
>         else if WARN_ON(__e && !IS_ERR_VALUE(__e))
>                 __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
>         (void *)__e;

Yeh that's nicer thanks.

>
> The check for constants may be fairly pointless.
> One of the static checkers may already detect the obvious fubar ERR_PTR(EINVAL).

True, but I figured it didn't add much overhead if we are placing
the runtime assertions anyway?

Thanks,
Amir.

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[David Laight]

On Fri, 15 May 2026 21:26:04 +0200
Amir Goldstein <amir73il@gmail.com> wrote:

> On Fri, May 15, 2026 at 8:30 PM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > On Thu, 14 May 2026 22:01:29 +0200
> > Amir Goldstein <amir73il@gmail.com> wrote:
> >  
...
> >
> > The object code bloat would be noticeable if this were used everywhere.
> > But you could make it a bit simpler:
> >         if (__builtin_constant_p(__e))
> >                 BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
> >         else if WARN_ON(__e && !IS_ERR_VALUE(__e))
> >                 __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
> >         (void *)__e;  
> 
> Yeh that's nicer thanks.

Actually this might be better still (or just more succinct):
	void *__e = (void *)error;
	BUILD_BUG_ON(!statically_true(IS_ERR_OR_NULL(__e));
	if (WARN_ON(!IS_ERR_OR_NULL(__e))
		__e = (void *)-EINVAL;
	__e;

The WARN_ON() will be optimised away (valid) constants.

> > The check for constants may be fairly pointless.
> > One of the static checkers may already detect the obvious fubar ERR_PTR(EINVAL).  
> 
> True, but I figured it didn't add much overhead if we are placing
> the runtime assertions anyway?

The 'size' check in READ_ONCE() measurably slows down the kernel compilation.
But there are a lot of those.
I think I know the main reason - the extern for the 'error message function'.

-- David

> 
> Thanks,
> Amir.

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Amir Goldstein]

On Sat, May 16, 2026 at 10:42 AM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Fri, 15 May 2026 21:26:04 +0200
> Amir Goldstein <amir73il@gmail.com> wrote:
>
> > On Fri, May 15, 2026 at 8:30 PM David Laight
> > <david.laight.linux@gmail.com> wrote:
> > >
> > > On Thu, 14 May 2026 22:01:29 +0200
> > > Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> ...
> > >
> > > The object code bloat would be noticeable if this were used everywhere.
> > > But you could make it a bit simpler:
> > >         if (__builtin_constant_p(__e))
> > >                 BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
> > >         else if WARN_ON(__e && !IS_ERR_VALUE(__e))
> > >                 __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
> > >         (void *)__e;
> >
> > Yeh that's nicer thanks.
>
> Actually this might be better still (or just more succinct):
>         void *__e = (void *)error;
>         BUILD_BUG_ON(!statically_true(IS_ERR_OR_NULL(__e));

This condition is wrong but also my compiler does not evaluate
__builtin_constant_p(IS_ERR_OR_NULL(__e)) as true.

This works
BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e)));

I think it is enough to statically assert on ERR_PTR(EINVAL)
and no need to bother with ERR_PTR(0)

>         if (WARN_ON(!IS_ERR_OR_NULL(__e))
>                 __e = (void *)-EINVAL;

Oh, anything but EINVAL please - the most overloaded error value
My choice of meaningful error value would be EFAULT
because without the safe helper we would be returning an address
which is in most likelihood bad, so better be explicit about it.

>         __e;
>
> The WARN_ON() will be optimised away (valid) constants.
>

Yeh this looks nice I'll use this:

#define ERR_PTR_SAFE(error) ({                                  \
        void *__e = (void *)(long)(error);                      \
        BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e)));      \
        if (WARN_ON(!IS_ERR_OR_NULL(__e)))                      \
                __e = (void *)(long)-EFAULT;                    \
        __e;                                                    \
})


Thanks!
Amir.

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[David Laight]

On Sat, 16 May 2026 13:39:11 +0200
Amir Goldstein <amir73il@gmail.com> wrote:

> On Sat, May 16, 2026 at 10:42 AM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > On Fri, 15 May 2026 21:26:04 +0200
> > Amir Goldstein <amir73il@gmail.com> wrote:
> >  
> > > On Fri, May 15, 2026 at 8:30 PM David Laight
> > > <david.laight.linux@gmail.com> wrote:  
> > > >
> > > > On Thu, 14 May 2026 22:01:29 +0200
> > > > Amir Goldstein <amir73il@gmail.com> wrote:
> > > >  
> > ...  
> > > >
> > > > The object code bloat would be noticeable if this were used everywhere.
> > > > But you could make it a bit simpler:
> > > >         if (__builtin_constant_p(__e))
> > > >                 BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
> > > >         else if WARN_ON(__e && !IS_ERR_VALUE(__e))
> > > >                 __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
> > > >         (void *)__e;  
> > >
> > > Yeh that's nicer thanks.  
> >
> > Actually this might be better still (or just more succinct):
> >         void *__e = (void *)error;
> >         BUILD_BUG_ON(!statically_true(IS_ERR_OR_NULL(__e));  
> 
> This condition is wrong but also my compiler does not evaluate
> __builtin_constant_p(IS_ERR_OR_NULL(__e)) as true.
> 
> This works
> BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e)));

Yes, it is easy to get those wrong - especially when typing quickly.

> 
> I think it is enough to statically assert on ERR_PTR(EINVAL)
> and no need to bother with ERR_PTR(0)

Then the tests don't match - which looks funny.
IS_ERR_VALUE(val) should be: val += 4095; jump_carry ...
and IS_ERR_OR_NULL(val): val--; val += 4096; jump_carry ...
but I can't remember whether gcc manages to do that.

> 
> >         if (WARN_ON(!IS_ERR_OR_NULL(__e))
> >                 __e = (void *)-EINVAL;  
> 
> Oh, anything but EINVAL please - the most overloaded error value
> My choice of meaningful error value would be EFAULT
> because without the safe helper we would be returning an address
> which is in most likelihood bad, so better be explicit about it.

I'm not sure about EFAULT; it is only really used for user copy failures.
IIRC at least one Unix (I've forgotten which) generates SIGSEGV when a
system call return of EFAULT.

There is also the 'problem' of PANIC_ON_WARN which is set by a lot
of distributions.
That (sort of) means than you might as well use BUG_ON() and get the
associated slightly smaller code size.
On x86-64 (and maybe a few others) both BUG_ON() and WARN_ON() just
execute UD2 (an undefined instruction) and the trap handler finds the
associated info and does the printk().
That makes the code smaller than pr_warn().
Someone needs to add a 'I_REALLY_MEAN_WARN_ON()' that never panics.
(And maybe with an option to not dump all the stack.)

-- David

> 
> >         __e;
> >
> > The WARN_ON() will be optimised away (valid) constants.
> >  
> 
> Yeh this looks nice I'll use this:
> 
> #define ERR_PTR_SAFE(error) ({                                  \
>         void *__e = (void *)(long)(error);                      \
>         BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e)));      \
>         if (WARN_ON(!IS_ERR_OR_NULL(__e)))                      \
>                 __e = (void *)(long)-EFAULT;                    \
>         __e;                                                    \
> })
> 
> 
> Thanks!
> Amir.

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Amir Goldstein]

On Sat, May 16, 2026 at 2:42 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Sat, 16 May 2026 13:39:11 +0200
> Amir Goldstein <amir73il@gmail.com> wrote:
>
> > On Sat, May 16, 2026 at 10:42 AM David Laight
> > <david.laight.linux@gmail.com> wrote:
> > >
> > > On Fri, 15 May 2026 21:26:04 +0200
> > > Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > > On Fri, May 15, 2026 at 8:30 PM David Laight
> > > > <david.laight.linux@gmail.com> wrote:
> > > > >
> > > > > On Thu, 14 May 2026 22:01:29 +0200
> > > > > Amir Goldstein <amir73il@gmail.com> wrote:
> > > > >
> > > ...
> > > > >
> > > > > The object code bloat would be noticeable if this were used everywhere.
> > > > > But you could make it a bit simpler:
> > > > >         if (__builtin_constant_p(__e))
> > > > >                 BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
> > > > >         else if WARN_ON(__e && !IS_ERR_VALUE(__e))
> > > > >                 __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
> > > > >         (void *)__e;
> > > >
> > > > Yeh that's nicer thanks.
> > >
> > > Actually this might be better still (or just more succinct):
> > >         void *__e = (void *)error;
> > >         BUILD_BUG_ON(!statically_true(IS_ERR_OR_NULL(__e));
> >
> > This condition is wrong but also my compiler does not evaluate
> > __builtin_constant_p(IS_ERR_OR_NULL(__e)) as true.
> >
> > This works
> > BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e)));
>
> Yes, it is easy to get those wrong - especially when typing quickly.
>
> >
> > I think it is enough to statically assert on ERR_PTR(EINVAL)
> > and no need to bother with ERR_PTR(0)
>
> Then the tests don't match - which looks funny.
> IS_ERR_VALUE(val) should be: val += 4095; jump_carry ...
> and IS_ERR_OR_NULL(val): val--; val += 4096; jump_carry ...
> but I can't remember whether gcc manages to do that.
>
> >
> > >         if (WARN_ON(!IS_ERR_OR_NULL(__e))
> > >                 __e = (void *)-EINVAL;
> >
> > Oh, anything but EINVAL please - the most overloaded error value
> > My choice of meaningful error value would be EFAULT
> > because without the safe helper we would be returning an address
> > which is in most likelihood bad, so better be explicit about it.
>
> I'm not sure about EFAULT; it is only really used for user copy failures.
> IIRC at least one Unix (I've forgotten which) generates SIGSEGV when a
> system call return of EFAULT.
>
> There is also the 'problem' of PANIC_ON_WARN which is set by a lot
> of distributions.
> That (sort of) means than you might as well use BUG_ON() and get the
> associated slightly smaller code size.

Come on. For real?
I might as well use BUG_ON() which is strictly and rightfully
frowned upon instead of using WARN_ON() because some distros
choose to do PANIC_ON_WARN?
That logic is backwards.

> On x86-64 (and maybe a few others) both BUG_ON() and WARN_ON() just
> execute UD2 (an undefined instruction) and the trap handler finds the
> associated info and does the printk().
> That makes the code smaller than pr_warn().
> Someone needs to add a 'I_REALLY_MEAN_WARN_ON()' that never panics.
> (And maybe with an option to not dump all the stack.)
>

Well, the whole point is that I would want to get a stack dump
if my code was doing ERR_PTR(<positive_value>) because without
this stack dump it would have been really hard to understand
the conditions that caused the regression in the Link.

Thanks,
Amir.

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Rasmus Villemoes]

On Sat, May 16 2026, Amir Goldstein <amir73il@gmail.com> wrote:

> On Sat, May 16, 2026 at 10:42 AM David Laight
> <david.laight.linux@gmail.com> wrote:
>>
>> On Fri, 15 May 2026 21:26:04 +0200
>> Amir Goldstein <amir73il@gmail.com> wrote:
>>
>> > On Fri, May 15, 2026 at 8:30 PM David Laight
>> > <david.laight.linux@gmail.com> wrote:
>> > >
>> > > On Thu, 14 May 2026 22:01:29 +0200
>> > > Amir Goldstein <amir73il@gmail.com> wrote:
>> > >
>> ...
>> > >
>> > > The object code bloat would be noticeable if this were used everywhere.
>> > > But you could make it a bit simpler:
>> > >         if (__builtin_constant_p(__e))
>> > >                 BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
>> > >         else if WARN_ON(__e && !IS_ERR_VALUE(__e))
>> > >                 __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
>> > >         (void *)__e;
>> >
>> > Yeh that's nicer thanks.
>>
>> Actually this might be better still (or just more succinct):
>>         void *__e = (void *)error;
>>         BUILD_BUG_ON(!statically_true(IS_ERR_OR_NULL(__e));
>
> This condition is wrong but also my compiler does not evaluate
> __builtin_constant_p(IS_ERR_OR_NULL(__e)) as true.
>
> This works
> BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e)));
>
> I think it is enough to statically assert on ERR_PTR(EINVAL)
> and no need to bother with ERR_PTR(0)
>
>>         if (WARN_ON(!IS_ERR_OR_NULL(__e))
>>                 __e = (void *)-EINVAL;
>
> Oh, anything but EINVAL please - the most overloaded error value
> My choice of meaningful error value would be EFAULT

Could we have a dedicated "EBUG" that can be used to indicate "there's a
bug somewhere in the kernel, we can handle it somewhat gracefully here
by returning an error instead of BUG(). You can't do anything about it
but report that you got -EBUG from $this_syscall", instead of
overloading EIO, EFAULT, EINVAL or whatnot. Internally, EBUG could be a
macro that did ({ WARN_ONCE(); -__EBUG; }) or something, so there would
automatically be bread crumbs in dmesg if it is ever hit.

Userspace code could also benefit from having EBUG to indicate that some
internal inconsistency has been detected.

Rasmus

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Amir Goldstein]

On Mon, May 18, 2026 at 11:04 AM Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On Sat, May 16 2026, Amir Goldstein <amir73il@gmail.com> wrote:
>
> > On Sat, May 16, 2026 at 10:42 AM David Laight
> > <david.laight.linux@gmail.com> wrote:
> >>
> >> On Fri, 15 May 2026 21:26:04 +0200
> >> Amir Goldstein <amir73il@gmail.com> wrote:
> >>
> >> > On Fri, May 15, 2026 at 8:30 PM David Laight
> >> > <david.laight.linux@gmail.com> wrote:
> >> > >
> >> > > On Thu, 14 May 2026 22:01:29 +0200
> >> > > Amir Goldstein <amir73il@gmail.com> wrote:
> >> > >
> >> ...
> >> > >
> >> > > The object code bloat would be noticeable if this were used everywhere.
> >> > > But you could make it a bit simpler:
> >> > >         if (__builtin_constant_p(__e))
> >> > >                 BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
> >> > >         else if WARN_ON(__e && !IS_ERR_VALUE(__e))
> >> > >                 __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
> >> > >         (void *)__e;
> >> >
> >> > Yeh that's nicer thanks.
> >>
> >> Actually this might be better still (or just more succinct):
> >>         void *__e = (void *)error;
> >>         BUILD_BUG_ON(!statically_true(IS_ERR_OR_NULL(__e));
> >
> > This condition is wrong but also my compiler does not evaluate
> > __builtin_constant_p(IS_ERR_OR_NULL(__e)) as true.
> >
> > This works
> > BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e)));
> >
> > I think it is enough to statically assert on ERR_PTR(EINVAL)
> > and no need to bother with ERR_PTR(0)
> >
> >>         if (WARN_ON(!IS_ERR_OR_NULL(__e))
> >>                 __e = (void *)-EINVAL;
> >
> > Oh, anything but EINVAL please - the most overloaded error value
> > My choice of meaningful error value would be EFAULT
>
> Could we have a dedicated "EBUG" that can be used to indicate "there's a
> bug somewhere in the kernel, we can handle it somewhat gracefully here
> by returning an error instead of BUG(). You can't do anything about it
> but report that you got -EBUG from $this_syscall", instead of
> overloading EIO, EFAULT, EINVAL or whatnot. Internally, EBUG could be a
> macro that did ({ WARN_ONCE(); -__EBUG; }) or something, so there would
> automatically be bread crumbs in dmesg if it is ever hit.

How exactly would you use this EBUG() to write the graceful handling?
A bit of overengineering if you ask me.

>
> Userspace code could also benefit from having EBUG to indicate that some
> internal inconsistency has been detected.

So what about

#define EBUG MAX_ERRNO

as per my suggested patch.

Thanks,
Amir.

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[David Laight]

On Mon, 18 May 2026 11:52:06 +0200
Amir Goldstein <amir73il@gmail.com> wrote:

> On Mon, May 18, 2026 at 11:04 AM Rasmus Villemoes <ravi@prevas.dk> wrote:
> >
> > On Sat, May 16 2026, Amir Goldstein <amir73il@gmail.com> wrote:
> >  
> > > On Sat, May 16, 2026 at 10:42 AM David Laight
> > > <david.laight.linux@gmail.com> wrote:  
> > >>
> > >> On Fri, 15 May 2026 21:26:04 +0200
> > >> Amir Goldstein <amir73il@gmail.com> wrote:
> > >>  
> > >> > On Fri, May 15, 2026 at 8:30 PM David Laight
> > >> > <david.laight.linux@gmail.com> wrote:  
> > >> > >
> > >> > > On Thu, 14 May 2026 22:01:29 +0200
> > >> > > Amir Goldstein <amir73il@gmail.com> wrote:
> > >> > >  
> > >> ...  
> > >> > >
> > >> > > The object code bloat would be noticeable if this were used everywhere.
> > >> > > But you could make it a bit simpler:
> > >> > >         if (__builtin_constant_p(__e))
> > >> > >                 BUILD_BUG_ON(__e && !IS_ERR_VALUE(__e));
> > >> > >         else if WARN_ON(__e && !IS_ERR_VALUE(__e))
> > >> > >                 __e = -MAX_ERRNO; // Or maybe -EINVAL to stop and other boundary errors
> > >> > >         (void *)__e;  
> > >> >
> > >> > Yeh that's nicer thanks.  
> > >>
> > >> Actually this might be better still (or just more succinct):
> > >>         void *__e = (void *)error;
> > >>         BUILD_BUG_ON(!statically_true(IS_ERR_OR_NULL(__e));  
> > >
> > > This condition is wrong but also my compiler does not evaluate
> > > __builtin_constant_p(IS_ERR_OR_NULL(__e)) as true.
> > >
> > > This works
> > > BUILD_BUG_ON(statically_true(!IS_ERR_VALUE(__e)));
> > >
> > > I think it is enough to statically assert on ERR_PTR(EINVAL)
> > > and no need to bother with ERR_PTR(0)
> > >  
> > >>         if (WARN_ON(!IS_ERR_OR_NULL(__e))
> > >>                 __e = (void *)-EINVAL;  
> > >
> > > Oh, anything but EINVAL please - the most overloaded error value
> > > My choice of meaningful error value would be EFAULT  
> >
> > Could we have a dedicated "EBUG" that can be used to indicate "there's a
> > bug somewhere in the kernel, we can handle it somewhat gracefully here
> > by returning an error instead of BUG(). You can't do anything about it
> > but report that you got -EBUG from $this_syscall", instead of
> > overloading EIO, EFAULT, EINVAL or whatnot. Internally, EBUG could be a
> > macro that did ({ WARN_ONCE(); -__EBUG; }) or something, so there would
> > automatically be bread crumbs in dmesg if it is ever hit.  
> 
> How exactly would you use this EBUG() to write the graceful handling?
> A bit of overengineering if you ask me.
> 
> >
> > Userspace code could also benefit from having EBUG to indicate that some
> > internal inconsistency has been detected.  
> 
> So what about
> 
> #define EBUG MAX_ERRNO
> 
> as per my suggested patch.

Your are relying on there being no 'off-by-one' errors.
They happen...

-- David

> 
> Thanks,
> Amir.
> 

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Andreas Dilger]

On Thu, 14 May 2026 22:01:29 +0200 Amir Goldstein <amir73il@gmail.com> wrote:
> 
> The check for constants may be fairly pointless.
> One of the static checkers may already detect the obvious fubar ERR_PTR(EINVAL).  


Actually, I just ran across an issue that checkpatch.pl does *not* detect
this "obvious" case.  It complains about "return EINVAL", but does not say
anything for cases like "return ERR_PTR(EINVAL)" or "rc = EINVAL; return rc;".

The following patch fixes checkpatch.pl to report many more such cases, and
has very few false positives for checking common error return assignments.

diff --git a/contrib/scripts/checkpatch.pl b/contrib/scripts/checkpatch.pl
index 70c78a3..e3fdedf 100755
--- a/contrib/scripts/checkpatch.pl
+++ b/contrib/scripts/checkpatch.pl
@@ -5795,11 +5795,12 @@
  		}
 
 # Return of what appears to be an errno should normally be negative
-		if ($sline =~ /\breturn(?:\s*\(+\s*|\s+)(E[A-Z]+)(?:\s*\)+\s*|\s*)[;:,]/) {
-			my $name = $1;
+		if (!is_userspace($realfile) &&
+		    $sline =~ /\b(?i)(return|err =|rc =|ret =|retval =|ERR_PTR)(?-i)(?:\s*\(+\s*|\s+)(E[A-Z]+)(?:\s*\)+\s*|\s*)[;:,]/) {
+			my $name = $2;
 			if ($name ne 'EOF' && $name ne 'ERROR' && $name !~ /^EPOLL/) {
 				WARN("USE_NEGATIVE_ERRNO",
-				     "return of an errno should typically be negative (ie: return -$1)\n" . $herecurr);
+				     "return of an errno should typically be negative (ie: $1 -$2)\n" . $herecurr);
 	}
 }
 

Cheers, Andreas

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[David Laight]

On Sun, 17 May 2026 03:13:00 -0600
Andreas Dilger <adilger@dilger.ca> wrote:

> On Thu, 14 May 2026 22:01:29 +0200 Amir Goldstein <amir73il@gmail.com> wrote:
> > 
> > The check for constants may be fairly pointless.
> > One of the static checkers may already detect the obvious fubar ERR_PTR(EINVAL).    
> 
> 
> Actually, I just ran across an issue that checkpatch.pl does *not* detect
> this "obvious" case.  It complains about "return EINVAL", but does not say
> anything for cases like "return ERR_PTR(EINVAL)" or "rc = EINVAL; return rc;".
> 
> The following patch fixes checkpatch.pl to report many more such cases, and
> has very few false positives for checking common error return assignments.

Looks like there are a few too many false positives in the network stack.
Mostly because sk_err holds a positive errno.
They do all seem to be 'err = Exxx' though.

Does look more useful that some of the other things that checkpatch checks.

-- David

> 
> diff --git a/contrib/scripts/checkpatch.pl b/contrib/scripts/checkpatch.pl
> index 70c78a3..e3fdedf 100755
> --- a/contrib/scripts/checkpatch.pl
> +++ b/contrib/scripts/checkpatch.pl
> @@ -5795,11 +5795,12 @@
>   		}
>  
>  # Return of what appears to be an errno should normally be negative
> -		if ($sline =~ /\breturn(?:\s*\(+\s*|\s+)(E[A-Z]+)(?:\s*\)+\s*|\s*)[;:,]/) {
> -			my $name = $1;
> +		if (!is_userspace($realfile) &&
> +		    $sline =~ /\b(?i)(return|err =|rc =|ret =|retval =|ERR_PTR)(?-i)(?:\s*\(+\s*|\s+)(E[A-Z]+)(?:\s*\)+\s*|\s*)[;:,]/) {
> +			my $name = $2;
>  			if ($name ne 'EOF' && $name ne 'ERROR' && $name !~ /^EPOLL/) {
>  				WARN("USE_NEGATIVE_ERRNO",
> -				     "return of an errno should typically be negative (ie: return -$1)\n" . $herecurr);
> +				     "return of an errno should typically be negative (ie: $1 -$2)\n" . $herecurr);
>  	}
>  }
>  
> 
> Cheers, Andreas
> 
> 
> 
> 
> 

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Christian Brauner]

On Thu, May 14, 2026 at 10:01:29PM +0200, Amir Goldstein wrote:
> Code using ERR_PTR() is almost certainly intending to produce a value
> which qualified as IS_ERR_OR_NULL(), but this is not the case when
> code calls ERR_PTR(err) with positive or large negative err.
> 
> Introduce a fortified variant of ERR_PTR() whose return value is
> guaranteed to qualify as IS_ERR_OR_NULL().
> 
> We add this in a new header file err_ptr.h which includes bug.h
> for the build/run time assertions.
> 
> Subsystems may opt-in for fortified ERR_PTR() for specific call sites
> or by #define ERR_PTR(err) ERR_PTR_SAFE(err).
> 
> Link: https://lore.kernel.org/r/CAOQ4uxg=gONUh5QEW5KJcyXLDF15HbLnc9Ea7RKPcgtyfPasTA@mail.gmail.com/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

I think this is backwards. You can of course do whatever you want in
overlayfs but I think as a first-class concept in err.h it's not that
great. Then we have to separate macros/inlines and almost no one will
use ERR_PTR_SAFE().

I think the correct thing would be to add an assert into ERR_PTR() and a
debug-only one very likely at that and combine this with the static
analyzer thing mentioned in the thread below.

diff --git a/include/linux/err.h b/include/linux/err.h
index 8c37be0620ab..6bf768adf157 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -38,6 +38,9 @@
  */
 static inline void * __must_check ERR_PTR(long error)
 {
+#ifdef CONFIG_DEBUG_INFO
+       WARN_ON_ONCE(!IS_ERR_VALUE(error));
+#endif
        return (void *) error;
 }

I'm not convinced yet about ERR_PTR_SAFE().

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[Amir Goldstein]

On Mon, May 18, 2026 at 2:39 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Thu, May 14, 2026 at 10:01:29PM +0200, Amir Goldstein wrote:
> > Code using ERR_PTR() is almost certainly intending to produce a value
> > which qualified as IS_ERR_OR_NULL(), but this is not the case when
> > code calls ERR_PTR(err) with positive or large negative err.
> >
> > Introduce a fortified variant of ERR_PTR() whose return value is
> > guaranteed to qualify as IS_ERR_OR_NULL().
> >
> > We add this in a new header file err_ptr.h which includes bug.h
> > for the build/run time assertions.
> >
> > Subsystems may opt-in for fortified ERR_PTR() for specific call sites
> > or by #define ERR_PTR(err) ERR_PTR_SAFE(err).
> >
> > Link: https://lore.kernel.org/r/CAOQ4uxg=gONUh5QEW5KJcyXLDF15HbLnc9Ea7RKPcgtyfPasTA@mail.gmail.com/
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
>
> I think this is backwards. You can of course do whatever you want in
> overlayfs but I think as a first-class concept in err.h it's not that
> great. Then we have to separate macros/inlines and almost no one will
> use ERR_PTR_SAFE().
>
> I think the correct thing would be to add an assert into ERR_PTR() and a
> debug-only one very likely at that and combine this with the static
> analyzer thing mentioned in the thread below.
>
> diff --git a/include/linux/err.h b/include/linux/err.h
> index 8c37be0620ab..6bf768adf157 100644
> --- a/include/linux/err.h
> +++ b/include/linux/err.h
> @@ -38,6 +38,9 @@
>   */
>  static inline void * __must_check ERR_PTR(long error)
>  {
> +#ifdef CONFIG_DEBUG_INFO
> +       WARN_ON_ONCE(!IS_ERR_VALUE(error));
> +#endif
>         return (void *) error;
>  }
>
> I'm not convinced yet about ERR_PTR_SAFE().

Maybe, but
1. err.h does not include bug.h, hence err_ptr.h
2. I think CONFIG_DEBUG_INFO is pretty common in distro kernels
and it means its ok to bloat the kernel object sizes
but it does not mean distro is willing to pay performance penalty
but David may be able to share more insight about the performance
cost of your suggested variant.

Thanks,
Amir.

Re: [PATCH] err_ptr.h: introduce ERR_PTR_SAFE()

[David Laight]

On Mon, 18 May 2026 14:52:05 +0200
Amir Goldstein <amir73il@gmail.com> wrote:

> On Mon, May 18, 2026 at 2:39 PM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Thu, May 14, 2026 at 10:01:29PM +0200, Amir Goldstein wrote:  
> > > Code using ERR_PTR() is almost certainly intending to produce a value
> > > which qualified as IS_ERR_OR_NULL(), but this is not the case when
> > > code calls ERR_PTR(err) with positive or large negative err.
> > >
> > > Introduce a fortified variant of ERR_PTR() whose return value is
> > > guaranteed to qualify as IS_ERR_OR_NULL().
> > >
> > > We add this in a new header file err_ptr.h which includes bug.h
> > > for the build/run time assertions.
> > >
> > > Subsystems may opt-in for fortified ERR_PTR() for specific call sites
> > > or by #define ERR_PTR(err) ERR_PTR_SAFE(err).
> > >
> > > Link: https://lore.kernel.org/r/CAOQ4uxg=gONUh5QEW5KJcyXLDF15HbLnc9Ea7RKPcgtyfPasTA@mail.gmail.com/
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---  
> >
> > I think this is backwards. You can of course do whatever you want in
> > overlayfs but I think as a first-class concept in err.h it's not that
> > great. Then we have to separate macros/inlines and almost no one will
> > use ERR_PTR_SAFE().
> >
> > I think the correct thing would be to add an assert into ERR_PTR() and a
> > debug-only one very likely at that and combine this with the static
> > analyzer thing mentioned in the thread below.
> >
> > diff --git a/include/linux/err.h b/include/linux/err.h
> > index 8c37be0620ab..6bf768adf157 100644
> > --- a/include/linux/err.h
> > +++ b/include/linux/err.h
> > @@ -38,6 +38,9 @@
> >   */
> >  static inline void * __must_check ERR_PTR(long error)
> >  {
> > +#ifdef CONFIG_DEBUG_INFO
> > +       WARN_ON_ONCE(!IS_ERR_VALUE(error));

If you do this, you need to change the value.
Even if just 'error | PAGE_MASK' (and check for zero).

You could just do:
	return error ? (void *)(error | ~4095ul) : NULL;
or (avoiding the conditional):
	return ((error - 1) | ~4095ul) + 1;

That will ensure the value is treated as an error value.

> > +#endif
> >         return (void *) error;
> >  }
> >
> > I'm not convinced yet about ERR_PTR_SAFE().  
> 
> Maybe, but
> 1. err.h does not include bug.h, hence err_ptr.h
> 2. I think CONFIG_DEBUG_INFO is pretty common in distro kernels
> and it means its ok to bloat the kernel object sizes
> but it does not mean distro is willing to pay performance penalty
> but David may be able to share more insight about the performance
> cost of your suggested variant.

The check itself (on x86) should just be 'add $4095, error; jnc bad'.
WARN_ON() and BUG_ON() are implemented using 'ud2' (undefined opcode)
and picking up the info from the exception table.
I can't remember about WARN_ON_ONCE() - there has to be some writable
global data somewhere.
But other architectures end up adding more in line code.
In particular anything that adds a function call can badly affect
register allocation and code generation.

Not the least of the problems is the sheer number of places where
extra code can get added - especially for something that static analysis
ought to be able to detect.

-- David

> 
> Thanks,
> Amir.