[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 */