[PATCH v1] Call prctl(2) with long integers, specify 5 arguments, and avoid casts

[PATCH v1] Call prctl(2) with long integers, specify 5 arguments, and avoid casts

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 11494 bytes --]

Since libc's prctl(2) wrapper is a variadic function, arguments must
have the right width.  Otherwise, the behavior is undefined.

Also, the 5 arguments must be specified always, or the behavior is also
undefined.  libc reads 5 values and passes them all to the kernel, so if
one is uninitialized, the kernel will receive garbagge, which could
result in EINVAL (most likely), or worse, a different action.

Also, avoid some casts to unsigned long, by changing the type of the
parameter to some local wrappers.

And use consistently 0L.  0UL is basically the same, and uses one more
character.  Keep it short.

Link: <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=6698b096a6f5342cb9b338c237ed875a8635497a>
Link: <https://lore.kernel.org/linux-man/ddbdyaiptesjalgfmztxideej67e3yaob7ucsmbf6qvriwxiif@dohhxrqgwhrf/T/#med306b5b003f9cc7cc2de69fcdd7ee2d056d0954>
Cc: Xi Ruoyao <xry111@xry111.site>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
Range-diff against v0:
-:  --------- > 1:  afd73139e Call prctl(2) with long integers, specify 5 arguments, and avoid casts

 include/seccomp.h            |  2 +-
 lib/caputils.c               |  4 ++--
 lib/env.c                    |  4 ++--
 misc-utils/enosys.c          |  4 ++--
 schedutils/coresched.c       |  6 +++---
 sys-utils/setpriv-landlock.c |  2 +-
 sys-utils/setpriv.c          | 34 ++++++++++++++++------------------
 tests/helpers/test_mkfds.c   |  2 +-
 8 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/include/seccomp.h b/include/seccomp.h
index 2b211439e..8c65b5e8c 100644
--- a/include/seccomp.h
+++ b/include/seccomp.h
@@ -18,7 +18,7 @@ static int ul_set_seccomp_filter_spec_allow(const struct sock_fprog *prog)
 		return 0;
 #endif
 
-	return prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, prog);
+	return prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, prog, 0L, 0L);
 }
 
 #endif /* UL_SECCOMP_H */
diff --git a/lib/caputils.c b/lib/caputils.c
index 23866c071..fd7037ff9 100644
--- a/lib/caputils.c
+++ b/lib/caputils.c
@@ -29,7 +29,7 @@
 static int test_cap(unsigned int cap)
 {
 	/* prctl returns 0 or 1 for valid caps, -1 otherwise */
-	return prctl(PR_CAPBSET_READ, cap, 0, 0, 0) >= 0;
+	return prctl(PR_CAPBSET_READ, cap, 0L, 0L, 0L) >= 0;
 }
 
 static int cap_last_by_bsearch(int *ret)
@@ -120,7 +120,7 @@ void cap_permitted_to_ambient(void)
 			continue;
 
 		if ((effective & (1ULL << cap))
-		    && prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, cap, 0, 0) < 0)
+		    && prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, cap, 0L, 0L) < 0)
 			err(EXIT_FAILURE, _("prctl(PR_CAP_AMBIENT) failed"));
 	}
 }
diff --git a/lib/env.c b/lib/env.c
index 2bdfe5697..5d9ab2456 100644
--- a/lib/env.c
+++ b/lib/env.c
@@ -191,11 +191,11 @@ char *safe_getenv(const char *arg)
 	if ((getuid() != geteuid()) || (getgid() != getegid()))
 		return NULL;
 #ifdef HAVE_PRCTL
-	if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 0)
+	if (prctl(PR_GET_DUMPABLE, 0L, 0L, 0L, 0L) == 0)
 		return NULL;
 #else
 #if (defined(linux) && defined(SYS_prctl))
-	if (syscall(SYS_prctl, PR_GET_DUMPABLE, 0, 0, 0, 0) == 0)
+	if (syscall(SYS_prctl, PR_GET_DUMPABLE, 0L, 0L, 0L, 0L) == 0)
 		return NULL;
 #endif
 #endif
diff --git a/misc-utils/enosys.c b/misc-utils/enosys.c
index 1410676dd..acf4428b6 100644
--- a/misc-utils/enosys.c
+++ b/misc-utils/enosys.c
@@ -290,10 +290,10 @@ int main(int argc, char **argv)
 	/* *SET* below will return EINVAL when either the filter is invalid or
 	 * seccomp is not supported. To distinguish those cases do a *GET* here
 	 */
-	if (prctl(PR_GET_SECCOMP) == -1 && errno == EINVAL)
+	if (prctl(PR_GET_SECCOMP, 0L, 0L, 0L, 0L) == -1 && errno == EINVAL)
 		err(EXIT_NOTSUPP, _("Seccomp non-functional"));
 
-	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0))
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L))
 		err_nosys(EXIT_FAILURE, _("Could not run prctl(PR_SET_NO_NEW_PRIVS)"));
 
 	if (ul_set_seccomp_filter_spec_allow(&prog))
diff --git a/schedutils/coresched.c b/schedutils/coresched.c
index 9d8be3e12..7844f98be 100644
--- a/schedutils/coresched.c
+++ b/schedutils/coresched.c
@@ -129,20 +129,20 @@ static sched_core_cookie core_sched_get_cookie(pid_t pid)
 
 static void core_sched_create_cookie(pid_t pid, sched_core_scope type)
 {
-	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, pid, type, 0))
+	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, pid, type, 0L))
 		err(EXIT_FAILURE, _("Failed to create cookie for PID %d"), pid);
 }
 
 static void core_sched_pull_cookie(pid_t from)
 {
 	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_FROM, from,
-		  PR_SCHED_CORE_SCOPE_THREAD, 0))
+		  PR_SCHED_CORE_SCOPE_THREAD, 0L))
 		err(EXIT_FAILURE, _("Failed to pull cookie from PID %d"), from);
 }
 
 static void core_sched_push_cookie(pid_t to, sched_core_scope type)
 {
-	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, to, type, 0))
+	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, to, type, 0L))
 		err(EXIT_FAILURE, _("Failed to push cookie to PID %d"), to);
 }
 
diff --git a/sys-utils/setpriv-landlock.c b/sys-utils/setpriv-landlock.c
index 00ad38c61..18c698325 100644
--- a/sys-utils/setpriv-landlock.c
+++ b/sys-utils/setpriv-landlock.c
@@ -187,7 +187,7 @@ void do_landlock(const struct setpriv_landlock_opts *opts)
 			err(SETPRIV_EXIT_PRIVERR, _("adding landlock rule failed"));
 	}
 
-	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1)
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L) == -1)
 		err(SETPRIV_EXIT_PRIVERR, _("disallow granting new privileges for landlock failed"));
 
 	if (landlock_restrict_self(fd, 0) == -1)
diff --git a/sys-utils/setpriv.c b/sys-utils/setpriv.c
index 4b0543101..0db09bf74 100644
--- a/sys-utils/setpriv.c
+++ b/sys-utils/setpriv.c
@@ -165,7 +165,7 @@ static void __attribute__((__noreturn__)) usage(void)
 	exit(EXIT_SUCCESS);
 }
 
-static int has_cap(enum cap_type which, unsigned int i)
+static int has_cap(enum cap_type which, unsigned long i)
 {
 	switch (which) {
 	case CAP_TYPE_EFFECTIVE:
@@ -174,8 +174,7 @@ static int has_cap(enum cap_type which, unsigned int i)
 	case CAP_TYPE_PERMITTED:
 		return capng_have_capability((capng_type_t)which, i);
 	case CAP_TYPE_AMBIENT:
-		return prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET,
-				(unsigned long) i, 0UL, 0UL);
+		return prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET, i, 0L, 0L);
 	default:
 		warnx(_("invalid capability type"));
 		return -1;
@@ -223,7 +222,7 @@ static void dump_one_secbit(int *first, int *bits, int bit, const char *name)
 static void dump_securebits(void)
 {
 	int first = 1;
-	int bits = prctl(PR_GET_SECUREBITS, 0, 0, 0, 0);
+	int bits = prctl(PR_GET_SECUREBITS, 0L, 0L, 0L, 0L);
 
 	if (bits < 0) {
 		warnx(_("getting process secure bits failed"));
@@ -323,7 +322,7 @@ static void dump_pdeathsig(void)
 {
 	int pdeathsig;
 
-	if (prctl(PR_GET_PDEATHSIG, &pdeathsig) != 0) {
+	if (prctl(PR_GET_PDEATHSIG, &pdeathsig, 0L, 0L, 0L) != 0) {
 		warn(_("get pdeathsig failed"));
 		return;
 	}
@@ -363,7 +362,7 @@ static void dump(int dumplevel)
 
 	dump_groups();
 
-	x = prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0);
+	x = prctl(PR_GET_NO_NEW_PRIVS, 0L, 0L, 0L, 0L);
 	if (0 <= x)
 		printf("no_new_privs: %d\n", x);
 	else
@@ -449,7 +448,7 @@ static void parse_groups(struct privctx *opts, const char *str)
 static void parse_pdeathsig(struct privctx *opts, const char *str)
 {
 	if (!strcmp(str, "keep")) {
-		if (prctl(PR_GET_PDEATHSIG, &opts->pdeathsig) != 0)
+		if (prctl(PR_GET_PDEATHSIG, &opts->pdeathsig, 0L, 0L, 0L) != 0)
 			errx(SETPRIV_EXIT_PRIVERR,
 				 _("failed to get parent death signal"));
 	} else if (!strcmp(str, "clear")) {
@@ -495,8 +494,7 @@ static void bump_cap(unsigned int cap)
 		capng_update(CAPNG_ADD, CAPNG_EFFECTIVE, cap);
 }
 
-static int cap_update(capng_act_t action,
-		enum cap_type type, unsigned int cap)
+static int cap_update(capng_act_t action, enum cap_type type, unsigned long cap)
 {
 	switch (type) {
 		case CAP_TYPE_EFFECTIVE:
@@ -510,10 +508,10 @@ static int cap_update(capng_act_t action,
 
 			if (action == CAPNG_ADD)
 				ret = prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE,
-						(unsigned long) cap, 0UL, 0UL);
+						cap, 0L, 0L);
 			else
 				ret = prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_LOWER,
-						(unsigned long) cap, 0UL, 0UL);
+						cap, 0L, 0L);
 
 			return ret;
 		}
@@ -565,7 +563,7 @@ static void parse_securebits(struct privctx *opts, const char *arg)
 	char *c;
 
 	opts->have_securebits = 1;
-	opts->securebits = prctl(PR_GET_SECUREBITS, 0, 0, 0, 0);
+	opts->securebits = prctl(PR_GET_SECUREBITS, 0L, 0L, 0L, 0L);
 	if (opts->securebits < 0)
 		err(SETPRIV_EXIT_PRIVERR, _("getting process secure bits failed"));
 
@@ -687,10 +685,10 @@ static void do_seccomp_filter(const char *file)
 	/* *SET* below will return EINVAL when either the filter is invalid or
 	 * seccomp is not supported. To distinguish those cases do a *GET* here
 	 */
-	if (prctl(PR_GET_SECCOMP) == -1 && errno == EINVAL)
+	if (prctl(PR_GET_SECCOMP, 0L, 0L, 0L, 0L) == -1 && errno == EINVAL)
 		err(SETPRIV_EXIT_PRIVERR, _("Seccomp non-functional"));
 
-	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0))
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L))
 		err(SETPRIV_EXIT_PRIVERR, _("Could not run prctl(PR_SET_NO_NEW_PRIVS)"));
 
 	if (ul_set_seccomp_filter_spec_allow(&prog))
@@ -1059,7 +1057,7 @@ int main(int argc, char **argv)
 		do_reset_environ(pw);
 	}
 
-	if (opts.nnp && prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1)
+	if (opts.nnp && prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L) == -1)
 		err(EXIT_FAILURE, _("disallow granting new privileges failed"));
 
 	if (opts.selinux_label)
@@ -1069,7 +1067,7 @@ int main(int argc, char **argv)
 	if (opts.seccomp_filter)
 		do_seccomp_filter(opts.seccomp_filter);
 
-	if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1)
+	if (prctl(PR_SET_KEEPCAPS, 1L, 0L, 0L, 0L) == -1)
 		err(EXIT_FAILURE, _("keep process capabilities failed"));
 
 	/* We're going to want CAP_SETPCAP, CAP_SETUID, and CAP_SETGID if
@@ -1102,7 +1100,7 @@ int main(int argc, char **argv)
 			err(SETPRIV_EXIT_PRIVERR, _("setgroups failed"));
 	}
 
-	if (opts.have_securebits && prctl(PR_SET_SECUREBITS, opts.securebits, 0, 0, 0) != 0)
+	if (opts.have_securebits && prctl(PR_SET_SECUREBITS, opts.securebits, 0L, 0L, 0L) != 0)
 		err(SETPRIV_EXIT_PRIVERR, _("set process securebits failed"));
 
 	if (opts.bounding_set) {
@@ -1123,7 +1121,7 @@ int main(int argc, char **argv)
 	}
 
 	/* Clear or set parent death signal */
-	if (opts.pdeathsig && prctl(PR_SET_PDEATHSIG, opts.pdeathsig < 0 ? 0 : opts.pdeathsig) != 0)
+	if (opts.pdeathsig && prctl(PR_SET_PDEATHSIG, opts.pdeathsig < 0 ? 0L : opts.pdeathsig, 0L, 0L, 0L) != 0)
 		err(SETPRIV_EXIT_PRIVERR, _("set parent death signal failed"));
 
 	do_landlock(&opts.landlock);
diff --git a/tests/helpers/test_mkfds.c b/tests/helpers/test_mkfds.c
index 60ebdd676..cfaa1f2ac 100644
--- a/tests/helpers/test_mkfds.c
+++ b/tests/helpers/test_mkfds.c
@@ -4372,7 +4372,7 @@ static void list_parameters(const char *factory_name)
 
 static void rename_self(const char *comm)
 {
-	if (prctl(PR_SET_NAME, (unsigned long)comm, 0, 0, 0) < 0)
+	if (prctl(PR_SET_NAME, (unsigned long)comm, 0L, 0L, 0L) < 0)
 		err(EXIT_FAILURE, "failed to rename self via prctl: %s", comm);
 }
 
-- 
2.45.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] Call prctl(2) with long integers, specify 5 arguments, and avoid casts

[Thomas Weißschuh]

On 2024-06-01 11:31:56+0000, Alejandro Colomar wrote:
> Since libc's prctl(2) wrapper is a variadic function, arguments must
> have the right width.  Otherwise, the behavior is undefined.

Ack.

> Also, the 5 arguments must be specified always, or the behavior is also
> undefined.  libc reads 5 values and passes them all to the kernel, so if
> one is uninitialized, the kernel will receive garbagge, which could
> result in EINVAL (most likely), or worse, a different action.

This seems surprising.

The kernel should only check the arguments it documents and not more.
glibc itself doesn't even specify all five arguments in its own calls to
prctl().

If all five arguments are really required then prctl() wouldn't need to
be variadic.

How is random non-zero data less valid than a essentially random zero?
And if the kernel actually validates this, how has it ever worked before?

Other popular software like systemd or opendjk also don't specify unused arguments.

So it doesn't really seem "broken".
If the patch is more about "being on the safe side", then this should be
spelled out.
(Plus the cases where documented, required arguments are missing)

> Also, avoid some casts to unsigned long, by changing the type of the
> parameter to some local wrappers.
> 
> And use consistently 0L.  0UL is basically the same, and uses one more
> character.  Keep it short.
> 
> Link: <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=6698b096a6f5342cb9b338c237ed875a8635497a>
> Link: <https://lore.kernel.org/linux-man/ddbdyaiptesjalgfmztxideej67e3yaob7ucsmbf6qvriwxiif@dohhxrqgwhrf/T/#med306b5b003f9cc7cc2de69fcdd7ee2d056d0954>
> Cc: Xi Ruoyao <xry111@xry111.site>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
> Range-diff against v0:
> -:  --------- > 1:  afd73139e Call prctl(2) with long integers, specify 5 arguments, and avoid casts
> 
>  include/seccomp.h            |  2 +-
>  lib/caputils.c               |  4 ++--
>  lib/env.c                    |  4 ++--
>  misc-utils/enosys.c          |  4 ++--
>  schedutils/coresched.c       |  6 +++---
>  sys-utils/setpriv-landlock.c |  2 +-
>  sys-utils/setpriv.c          | 34 ++++++++++++++++------------------
>  tests/helpers/test_mkfds.c   |  2 +-
>  8 files changed, 28 insertions(+), 30 deletions(-)

<snip>

Re: [PATCH v1] Call prctl(2) with long integers, specify 5 arguments, and avoid casts

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 6972 bytes --]

Hi Thomas,

On Sat, Jun 01, 2024 at 01:05:02PM GMT, Thomas Weißschuh wrote:
> On 2024-06-01 11:31:56+0000, Alejandro Colomar wrote:
> > Since libc's prctl(2) wrapper is a variadic function, arguments must
> > have the right width.  Otherwise, the behavior is undefined.
> 
> Ack.
> 
> > Also, the 5 arguments must be specified always, or the behavior is also
> > undefined.  libc reads 5 values and passes them all to the kernel, so if
> > one is uninitialized, the kernel will receive garbagge, which could
> > result in EINVAL (most likely), or worse, a different action.
> 
> This seems surprising.
> 
> The kernel should only check the arguments it documents and not more.

Hmmm, some prctl(2) calls don't document a need for passing 0 (probably
for legacy compatibility; you're right.  Only newer prctl(2)s check
those args.

And see for example these kernel commit:

	commit e9d1b4f3c60997fe197bf0243cb4a41a44387a88
	Author: Dave Hansen <dave.hansen@linux.intel.com>
	Date:   Thu Jan 8 14:30:22 2015 -0800

	    x86, mpx: Strictly enforce empty prctl() args
	    
	    Description from Michael Kerrisk.  He suggested an identical patch
	    to one I had already coded up and tested.
	    
	    commit fe3d197f8431 "x86, mpx: On-demand kernel allocation of bounds
	    tables" added two new prctl() operations, PR_MPX_ENABLE_MANAGEMENT and
	    PR_MPX_DISABLE_MANAGEMENT.  However, no checks were included to ensure
	    that unused arguments are zero, as is done in many existing prctl()s
	    and as should be done for all new prctl()s. This patch adds the
	    required checks.
	    
	    Suggested-by: Andy Lutomirski <luto@amacapital.net>
	    Suggested-by: Michael Kerrisk <mtk.manpages@gmail.com>
	    Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
	    Cc: Dave Hansen <dave@sr71.net>
	    Link: http://lkml.kernel.org/r/20150108223022.7F56FD13@viggo.jf.intel.com
	    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

	diff --git a/kernel/sys.c b/kernel/sys.c
	index a8c9f5a7dda6..ea9c88109894 100644
	--- a/kernel/sys.c
	+++ b/kernel/sys.c
	@@ -2210,9 +2210,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
			up_write(&me->mm->mmap_sem);
			break;
		case PR_MPX_ENABLE_MANAGEMENT:
	+               if (arg2 || arg3 || arg4 || arg5)
	+                       return -EINVAL;
			error = MPX_ENABLE_MANAGEMENT(me);
			break;
		case PR_MPX_DISABLE_MANAGEMENT:
	+               if (arg2 || arg3 || arg4 || arg5)
	+                       return -EINVAL;
			error = MPX_DISABLE_MANAGEMENT(me);
			break;
		default:

And this one too:

	commit 3e91ec89f527b9870fe42dcbdb74fd389d123a95
	Author: Catalin Marinas <catalin.marinas@arm.com>
	Date:   Thu Aug 15 16:44:00 2019 +0100

	    arm64: Tighten the PR_{SET, GET}_TAGGED_ADDR_CTRL prctl() unused arguments
	    
	    Require that arg{3,4,5} of the PR_{SET,GET}_TAGGED_ADDR_CTRL prctl and
	    arg2 of the PR_GET_TAGGED_ADDR_CTRL prctl() are zero rather than ignored
	    for future extensions.
	    
	    Acked-by: Andrey Konovalov <andreyknvl@google.com>
	    Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
	    Signed-off-by: Will Deacon <will@kernel.org>

	diff --git a/kernel/sys.c b/kernel/sys.c
	index c6c4d5358bd3..ec48396b4943 100644
	--- a/kernel/sys.c
	+++ b/kernel/sys.c
	@@ -2499,9 +2499,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
			error = PAC_RESET_KEYS(me, arg2);
			break;
		case PR_SET_TAGGED_ADDR_CTRL:
	+               if (arg3 || arg4 || arg5)
	+                       return -EINVAL;
			error = SET_TAGGED_ADDR_CTRL(arg2);
			break;
		case PR_GET_TAGGED_ADDR_CTRL:
	+               if (arg2 || arg3 || arg4 || arg5)
	+                       return -EINVAL;
			error = GET_TAGGED_ADDR_CTRL();
			break;
		default:

In the few calls that util-linux makes without specifying all 5 args,
the kernel seems to not do the checks (in some old prctl(2)s they didn't
have that check, and nobody seems to have cared enough to add it), so
it's more like we're lucky (or unlucky, depending on how you see it).

> glibc itself doesn't even specify all five arguments in its own calls to
> prctl().

glibc itself is wrong.  I'm even surprised that the PR_* macros from the
kernel UAPI for arg2 work without specifying the L suffix on them, but
it's probably just luck.

<https://lore.kernel.org/linux-api/20240528114750.106187-1-alx@kernel.org/T/#u>

> If all five arguments are really required then prctl() wouldn't need to
> be variadic.

Indeed.  I guess that's for historic reasons, rather than actual
necessity; but I don't know for sure.

> How is random non-zero data less valid than a essentially random zero?
> And if the kernel actually validates this, how has it ever worked before?

They only added validation for (all) new prctl(2) calls, plus maybe some
old ones, but not all.  In the ones used in util-linux that don't
specify zero, I've checked now that the kernel doesn't validate.

However, a call such as

	prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)

(this call exists in util-linux)
actually means

	prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, random, random)

and it supposedly has been working so far.  Those random bits are
probably 0 most of the time, for some reason.  And the kernel does check
this one:

	$ sed -n /PR_SET_NO_NEW_PRIVS/,+2p <kernel/sys.c
		case PR_SET_NO_NEW_PRIVS:
			if (arg2 != 1 || arg3 || arg4 || arg5)
				return -EINVAL;

> Other popular software like systemd or opendjk also don't specify unused arguments.

I've also checked that the ones that systemd uses without specifying all
5 args, they are not checked by the kernel.

> So it doesn't really seem "broken".
> If the patch is more about "being on the safe side", then this should be
> spelled out.

Still, libc reads those values (on x32) which results in Undefined
Behavior inside glibc.  Which is a bad thing.  Not broken, because the
compiler has little information to exploit that UB, but not a good thing
either.

	$ grepc __prctl .
	./include/sys/prctl.h:extern int __prctl (int __option, ...);
	./sysdeps/unix/sysv/linux/x86_64/x32/prctl.c:int
	__prctl (int option, ...)
	{
	  va_list arg;
	  va_start (arg, option);
	  unsigned long int arg2 = va_arg (arg, unsigned long int);
	  unsigned long int arg3 = va_arg (arg, unsigned long int);
	  unsigned long int arg4 = va_arg (arg, unsigned long int);
	  unsigned long int arg5 = va_arg (arg, unsigned long int);
	  va_end (arg);
	  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
	}

It's arguably less broken than the missing 'L', though.

> (Plus the cases where documented, required arguments are missing)

None of the cases where we omit the arguments are checked by the kernel.


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] Call prctl(2) with long integers, specify 5 arguments, and avoid casts

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 7673 bytes --]

On Sat, Jun 01, 2024 at 02:24:03PM GMT, Alejandro Colomar wrote:
> Hi Thomas,
> 
> On Sat, Jun 01, 2024 at 01:05:02PM GMT, Thomas Weißschuh wrote:
> > On 2024-06-01 11:31:56+0000, Alejandro Colomar wrote:
> > > Since libc's prctl(2) wrapper is a variadic function, arguments must
> > > have the right width.  Otherwise, the behavior is undefined.
> > 
> > Ack.
> > 
> > > Also, the 5 arguments must be specified always, or the behavior is also
> > > undefined.  libc reads 5 values and passes them all to the kernel, so if
> > > one is uninitialized, the kernel will receive garbagge, which could
> > > result in EINVAL (most likely), or worse, a different action.
> > 
> > This seems surprising.
> > 
> > The kernel should only check the arguments it documents and not more.
> 
> Hmmm, some prctl(2) calls don't document a need for passing 0 (probably
> for legacy compatibility; you're right.  Only newer prctl(2)s check
> those args.
> 
> And see for example these kernel commit:
> 
> 	commit e9d1b4f3c60997fe197bf0243cb4a41a44387a88
> 	Author: Dave Hansen <dave.hansen@linux.intel.com>
> 	Date:   Thu Jan 8 14:30:22 2015 -0800
> 
> 	    x86, mpx: Strictly enforce empty prctl() args
> 	    
> 	    Description from Michael Kerrisk.  He suggested an identical patch
> 	    to one I had already coded up and tested.
> 	    
> 	    commit fe3d197f8431 "x86, mpx: On-demand kernel allocation of bounds
> 	    tables" added two new prctl() operations, PR_MPX_ENABLE_MANAGEMENT and
> 	    PR_MPX_DISABLE_MANAGEMENT.  However, no checks were included to ensure
> 	    that unused arguments are zero, as is done in many existing prctl()s
> 	    and as should be done for all new prctl()s. This patch adds the
> 	    required checks.
> 	    
> 	    Suggested-by: Andy Lutomirski <luto@amacapital.net>
> 	    Suggested-by: Michael Kerrisk <mtk.manpages@gmail.com>
> 	    Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> 	    Cc: Dave Hansen <dave@sr71.net>
> 	    Link: http://lkml.kernel.org/r/20150108223022.7F56FD13@viggo.jf.intel.com
> 	    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> 	diff --git a/kernel/sys.c b/kernel/sys.c
> 	index a8c9f5a7dda6..ea9c88109894 100644
> 	--- a/kernel/sys.c
> 	+++ b/kernel/sys.c
> 	@@ -2210,9 +2210,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> 			up_write(&me->mm->mmap_sem);
> 			break;
> 		case PR_MPX_ENABLE_MANAGEMENT:
> 	+               if (arg2 || arg3 || arg4 || arg5)
> 	+                       return -EINVAL;
> 			error = MPX_ENABLE_MANAGEMENT(me);
> 			break;
> 		case PR_MPX_DISABLE_MANAGEMENT:
> 	+               if (arg2 || arg3 || arg4 || arg5)
> 	+                       return -EINVAL;
> 			error = MPX_DISABLE_MANAGEMENT(me);
> 			break;
> 		default:
> 
> And this one too:
> 
> 	commit 3e91ec89f527b9870fe42dcbdb74fd389d123a95
> 	Author: Catalin Marinas <catalin.marinas@arm.com>
> 	Date:   Thu Aug 15 16:44:00 2019 +0100
> 
> 	    arm64: Tighten the PR_{SET, GET}_TAGGED_ADDR_CTRL prctl() unused arguments
> 	    
> 	    Require that arg{3,4,5} of the PR_{SET,GET}_TAGGED_ADDR_CTRL prctl and
> 	    arg2 of the PR_GET_TAGGED_ADDR_CTRL prctl() are zero rather than ignored
> 	    for future extensions.
> 	    
> 	    Acked-by: Andrey Konovalov <andreyknvl@google.com>
> 	    Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> 	    Signed-off-by: Will Deacon <will@kernel.org>
> 
> 	diff --git a/kernel/sys.c b/kernel/sys.c
> 	index c6c4d5358bd3..ec48396b4943 100644
> 	--- a/kernel/sys.c
> 	+++ b/kernel/sys.c
> 	@@ -2499,9 +2499,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> 			error = PAC_RESET_KEYS(me, arg2);
> 			break;
> 		case PR_SET_TAGGED_ADDR_CTRL:
> 	+               if (arg3 || arg4 || arg5)
> 	+                       return -EINVAL;
> 			error = SET_TAGGED_ADDR_CTRL(arg2);
> 			break;
> 		case PR_GET_TAGGED_ADDR_CTRL:
> 	+               if (arg2 || arg3 || arg4 || arg5)
> 	+                       return -EINVAL;
> 			error = GET_TAGGED_ADDR_CTRL();
> 			break;
> 		default:
> 
> In the few calls that util-linux makes without specifying all 5 args,
> the kernel seems to not do the checks (in some old prctl(2)s they didn't
> have that check, and nobody seems to have cared enough to add it), so
> it's more like we're lucky (or unlucky, depending on how you see it).
> 
> > glibc itself doesn't even specify all five arguments in its own calls to
> > prctl().
> 
> glibc itself is wrong.  I'm even surprised that the PR_* macros from the
> kernel UAPI for arg2 work without specifying the L suffix on them, but
> it's probably just luck.
> 
> <https://lore.kernel.org/linux-api/20240528114750.106187-1-alx@kernel.org/T/#u>
> 
> > If all five arguments are really required then prctl() wouldn't need to
> > be variadic.
> 
> Indeed.  I guess that's for historic reasons, rather than actual
> necessity; but I don't know for sure.
> 
> > How is random non-zero data less valid than a essentially random zero?
> > And if the kernel actually validates this, how has it ever worked before?
> 
> They only added validation for (all) new prctl(2) calls, plus maybe some
> old ones, but not all.  In the ones used in util-linux that don't
> specify zero, I've checked now that the kernel doesn't validate.
> 
> However, a call such as
> 
> 	prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)
> 
> (this call exists in util-linux)
> actually means
> 
> 	prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, random, random)
> 
> and it supposedly has been working so far.  Those random bits are
> probably 0 most of the time, for some reason.  And the kernel does check
> this one:
> 
> 	$ sed -n /PR_SET_NO_NEW_PRIVS/,+2p <kernel/sys.c
> 		case PR_SET_NO_NEW_PRIVS:
> 			if (arg2 != 1 || arg3 || arg4 || arg5)
> 				return -EINVAL;
> 
> > Other popular software like systemd or opendjk also don't specify unused arguments.
> 
> I've also checked that the ones that systemd uses without specifying all
> 5 args, they are not checked by the kernel.
> 
> > So it doesn't really seem "broken".
> > If the patch is more about "being on the safe side", then this should be
> > spelled out.
> 
> Still, libc reads those values (on x32) which results in Undefined
> Behavior inside glibc.  Which is a bad thing.  Not broken, because the
> compiler has little information to exploit that UB, but not a good thing
> either.
> 
> 	$ grepc __prctl .
> 	./include/sys/prctl.h:extern int __prctl (int __option, ...);
> 	./sysdeps/unix/sysv/linux/x86_64/x32/prctl.c:int
> 	__prctl (int option, ...)
> 	{
> 	  va_list arg;
> 	  va_start (arg, option);
> 	  unsigned long int arg2 = va_arg (arg, unsigned long int);
> 	  unsigned long int arg3 = va_arg (arg, unsigned long int);
> 	  unsigned long int arg4 = va_arg (arg, unsigned long int);
> 	  unsigned long int arg5 = va_arg (arg, unsigned long int);
> 	  va_end (arg);
> 	  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
> 	}

Or one could say this is not a user problem, and just an implementation
detail of libc.  For those calls that the kernel ignores the argument,
omitting the argument sounds reasonable as a user.  I guess the kernel
won't expand those APIs, since that would be dangerous (for this precise
reason).

> 
> It's arguably less broken than the missing 'L', though.
> 
> > (Plus the cases where documented, required arguments are missing)
> 
> None of the cases where we omit the arguments are checked by the kernel.

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] Call prctl(2) with long integers, specify 5 arguments, and avoid casts

[Karel Zak]

On Sat, Jun 01, 2024 at 11:31:56AM +0200, Alejandro Colomar wrote:
> Since libc's prctl(2) wrapper is a variadic function, arguments must
> have the right width.  Otherwise, the behavior is undefined.

Created PR to test it by CI:
https://github.com/util-linux/util-linux/pull/3085

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

[PATCH v2 0/1] Call prctl(2) with signed long integers, and avoid casts

[Alejandro Colomar]

Hi!

Karel reminded me of this old patch.  Here's a revision of the patch.
Major changes in v2:

-  Don't specify the 5 arguments unnecessarily.

BTW, I've developed a header file that might be useful for the general
public.  See in a reply to this mail.


Have a lovely night!
Alex

Alejandro Colomar (1):
  c4fc41abf Call prctl(2) with signed long integers, and avoid casts

 lib/caputils.c               |  6 +++---
 lib/env.c                    |  4 ++--
 misc-utils/enosys.c          |  2 +-
 schedutils/coresched.c       |  6 +++---
 sys-utils/setpriv-landlock.c |  2 +-
 sys-utils/setpriv.c          | 27 +++++++++++++--------------
 tests/helpers/test_mkfds.c   |  2 +-
 7 files changed, 24 insertions(+), 25 deletions(-)
-- 
2.51.0

[PATCH v2 1/1] Call prctl(2) with signed long integers, and avoid casts

[Alejandro Colomar]

Since libc's prctl(2) wrapper is a variadic function, arguments must
have the right width.  Otherwise, the behavior is undefined.  The kernel
expects long arguments, so let's provide them.

Also, avoid some casts to unsigned long, by changing the type of the
parameter in some local wrappers.

And use consistently 0L.  0UL is basically the same, and uses one more
character.  Keep it short.  Also, unsigned integer literals are
dangerous, as the compiler can't diagnose mistakes such as overflow.

-  Casts are evil.
-  prctl(2) expects long integers, and

Link: <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=6698b096a6f5342cb9b338c237ed875a8635497a>
Link: <https://lore.kernel.org/linux-man/ddbdyaiptesjalgfmztxideej67e3yaob7ucsmbf6qvriwxiif@dohhxrqgwhrf/T/#med306b5b003f9cc7cc2de69fcdd7ee2d056d0954>
Link: <https://lore.kernel.org/util-linux/20240601093150.16912-1-alx@kernel.org/T/#u>
Link: <https://github.com/util-linux/util-linux/pull/3085>
Cc: Xi Ruoyao <xry111@xry111.site>
Cc: Thomas Weißschuh <thomas@t-8ch.de>
Cc: Karel Zak <kzak@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 lib/caputils.c               |  6 +++---
 lib/env.c                    |  4 ++--
 misc-utils/enosys.c          |  2 +-
 schedutils/coresched.c       |  6 +++---
 sys-utils/setpriv-landlock.c |  2 +-
 sys-utils/setpriv.c          | 27 +++++++++++++--------------
 tests/helpers/test_mkfds.c   |  2 +-
 7 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/lib/caputils.c b/lib/caputils.c
index 6c71c06b8..ea885ac62 100644
--- a/lib/caputils.c
+++ b/lib/caputils.c
@@ -25,10 +25,10 @@
 #include "procfs.h"
 #include "nls.h"
 
-static int test_cap(unsigned int cap)
+static int test_cap(unsigned long cap)
 {
 	/* prctl returns 0 or 1 for valid caps, -1 otherwise */
-	return prctl(PR_CAPBSET_READ, cap, 0, 0, 0) >= 0;
+	return prctl(PR_CAPBSET_READ, cap, 0L, 0L, 0L) >= 0;
 }
 
 static int cap_last_by_bsearch(int *ret)
@@ -119,7 +119,7 @@ void cap_permitted_to_ambient(void)
 			continue;
 
 		if ((effective & (1ULL << cap))
-		    && prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, cap, 0, 0) < 0)
+		    && prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, cap, 0L, 0L) < 0)
 			err(EXIT_FAILURE, _("prctl(PR_CAP_AMBIENT) failed"));
 	}
 }
diff --git a/lib/env.c b/lib/env.c
index 039fad0dc..7669a17b8 100644
--- a/lib/env.c
+++ b/lib/env.c
@@ -263,11 +263,11 @@ char *safe_getenv(const char *arg)
 	if (is_privileged_execution())
 		return NULL;
 #ifdef HAVE_PRCTL
-	if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 0)
+	if (prctl(PR_GET_DUMPABLE, 0L, 0L, 0L, 0L) == 0)
 		return NULL;
 #else
 #if (defined(linux) && defined(SYS_prctl))
-	if (syscall(SYS_prctl, PR_GET_DUMPABLE, 0, 0, 0, 0) == 0)
+	if (syscall(SYS_prctl, PR_GET_DUMPABLE, 0L, 0L, 0L, 0L) == 0)
 		return NULL;
 #endif
 #endif
diff --git a/misc-utils/enosys.c b/misc-utils/enosys.c
index f1438b8e8..11f183901 100644
--- a/misc-utils/enosys.c
+++ b/misc-utils/enosys.c
@@ -298,7 +298,7 @@ int main(int argc, char **argv)
 	if (prctl(PR_GET_SECCOMP) == -1 && errno == EINVAL)
 		err(EXIT_NOTSUPP, _("Seccomp non-functional"));
 
-	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0))
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L))
 		err_nosys(EXIT_FAILURE, _("Could not run prctl(PR_SET_NO_NEW_PRIVS)"));
 
 	if (ul_set_seccomp_filter_spec_allow(&prog))
diff --git a/schedutils/coresched.c b/schedutils/coresched.c
index 419745897..aaacf4027 100644
--- a/schedutils/coresched.c
+++ b/schedutils/coresched.c
@@ -122,20 +122,20 @@ static sched_core_cookie core_sched_get_cookie(pid_t pid)
 
 static void core_sched_create_cookie(pid_t pid, sched_core_scope type)
 {
-	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, pid, type, 0))
+	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, pid, type, 0L))
 		err(EXIT_FAILURE, _("Failed to create cookie for PID %d"), pid);
 }
 
 static void core_sched_pull_cookie(pid_t from)
 {
 	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_FROM, from,
-		  PR_SCHED_CORE_SCOPE_THREAD, 0))
+		  PR_SCHED_CORE_SCOPE_THREAD, 0L))
 		err(EXIT_FAILURE, _("Failed to pull cookie from PID %d"), from);
 }
 
 static void core_sched_push_cookie(pid_t to, sched_core_scope type)
 {
-	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, to, type, 0))
+	if (prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, to, type, 0L))
 		err(EXIT_FAILURE, _("Failed to push cookie to PID %d"), to);
 }
 
diff --git a/sys-utils/setpriv-landlock.c b/sys-utils/setpriv-landlock.c
index 90ab8954b..9089a3c95 100644
--- a/sys-utils/setpriv-landlock.c
+++ b/sys-utils/setpriv-landlock.c
@@ -191,7 +191,7 @@ void do_landlock(const struct setpriv_landlock_opts *opts)
 			err(SETPRIV_EXIT_PRIVERR, _("adding landlock rule failed"));
 	}
 
-	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1)
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L) == -1)
 		err(SETPRIV_EXIT_PRIVERR, _("disallow granting new privileges for landlock failed"));
 
 	if (landlock_restrict_self(fd, 0) == -1)
diff --git a/sys-utils/setpriv.c b/sys-utils/setpriv.c
index c218be8e5..f0f423819 100644
--- a/sys-utils/setpriv.c
+++ b/sys-utils/setpriv.c
@@ -171,7 +171,7 @@ static void __attribute__((__noreturn__)) usage(void)
 	exit(EXIT_SUCCESS);
 }
 
-static int has_cap(enum cap_type which, unsigned int i)
+static int has_cap(enum cap_type which, unsigned long i)
 {
 	switch (which) {
 	case CAP_TYPE_EFFECTIVE:
@@ -180,8 +180,7 @@ static int has_cap(enum cap_type which, unsigned int i)
 	case CAP_TYPE_PERMITTED:
 		return capng_have_capability((capng_type_t)which, i);
 	case CAP_TYPE_AMBIENT:
-		return prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET,
-				(unsigned long) i, 0UL, 0UL);
+		return prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET, i, 0L, 0L);
 	default:
 		warnx(_("invalid capability type"));
 		return -1;
@@ -229,7 +228,7 @@ static void dump_one_secbit(int *first, int *bits, int bit, const char *name)
 static void dump_securebits(void)
 {
 	int first = 1;
-	int bits = prctl(PR_GET_SECUREBITS, 0, 0, 0, 0);
+	int bits = prctl(PR_GET_SECUREBITS, 0L, 0L, 0L, 0L);
 
 	if (bits < 0) {
 		warnx(_("getting process secure bits failed"));
@@ -369,7 +368,7 @@ static void dump(int dumplevel)
 
 	dump_groups();
 
-	x = prctl(PR_GET_NO_NEW_PRIVS, 0, 0, 0, 0);
+	x = prctl(PR_GET_NO_NEW_PRIVS, 0L, 0L, 0L, 0L);
 	if (0 <= x)
 		printf("no_new_privs: %d\n", x);
 	else
@@ -513,7 +512,7 @@ static void bump_cap(unsigned int cap)
 }
 
 static int cap_update(capng_act_t action,
-		enum cap_type type, unsigned int cap)
+		enum cap_type type, unsigned long cap)
 {
 	switch (type) {
 		case CAP_TYPE_EFFECTIVE:
@@ -527,10 +526,10 @@ static int cap_update(capng_act_t action,
 
 			if (action == CAPNG_ADD)
 				ret = prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE,
-						(unsigned long) cap, 0UL, 0UL);
+						cap, 0L, 0L);
 			else
 				ret = prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_LOWER,
-						(unsigned long) cap, 0UL, 0UL);
+						cap, 0L, 0L);
 
 			return ret;
 		}
@@ -584,7 +583,7 @@ static void parse_securebits(struct privctx *opts, const char *arg)
 	char *c;
 
 	opts->have_securebits = 1;
-	opts->securebits = prctl(PR_GET_SECUREBITS, 0, 0, 0, 0);
+	opts->securebits = prctl(PR_GET_SECUREBITS, 0L, 0L, 0L, 0L);
 	if (opts->securebits < 0)
 		err(SETPRIV_EXIT_PRIVERR, _("getting process secure bits failed"));
 
@@ -709,7 +708,7 @@ static void do_seccomp_filter(const char *file)
 	if (prctl(PR_GET_SECCOMP) == -1 && errno == EINVAL)
 		err(SETPRIV_EXIT_PRIVERR, _("Seccomp non-functional"));
 
-	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0))
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L))
 		err(SETPRIV_EXIT_PRIVERR, _("Could not run prctl(PR_SET_NO_NEW_PRIVS)"));
 
 	if (ul_set_seccomp_filter_spec_allow(&prog))
@@ -1086,7 +1085,7 @@ int main(int argc, char **argv)
 		do_reset_environ(pw);
 	}
 
-	if (opts.nnp && prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1)
+	if (opts.nnp && prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L) == -1)
 		err(EXIT_FAILURE, _("disallow granting new privileges failed"));
 
 	if (opts.selinux_label)
@@ -1096,7 +1095,7 @@ int main(int argc, char **argv)
 	if (opts.seccomp_filter)
 		do_seccomp_filter(opts.seccomp_filter);
 
-	if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) == -1)
+	if (prctl(PR_SET_KEEPCAPS, 1L, 0L, 0L, 0L) == -1)
 		err(EXIT_FAILURE, _("keep process capabilities failed"));
 
 	/* We're going to want CAP_SETPCAP, CAP_SETUID, and CAP_SETGID if
@@ -1129,7 +1128,7 @@ int main(int argc, char **argv)
 			err(SETPRIV_EXIT_PRIVERR, _("setgroups failed"));
 	}
 
-	if (opts.have_securebits && prctl(PR_SET_SECUREBITS, opts.securebits, 0, 0, 0) != 0)
+	if (opts.have_securebits && prctl(PR_SET_SECUREBITS, opts.securebits, 0L, 0L, 0L) != 0)
 		err(SETPRIV_EXIT_PRIVERR, _("set process securebits failed"));
 
 	if (opts.bounding_set) {
@@ -1150,7 +1149,7 @@ int main(int argc, char **argv)
 	}
 
 	/* Clear or set parent death signal */
-	if (opts.pdeathsig && prctl(PR_SET_PDEATHSIG, opts.pdeathsig < 0 ? 0 : opts.pdeathsig) != 0)
+	if (opts.pdeathsig && prctl(PR_SET_PDEATHSIG, opts.pdeathsig < 0 ? 0L : opts.pdeathsig) != 0)
 		err(SETPRIV_EXIT_PRIVERR, _("set parent death signal failed"));
 
 	if (opts.have_ptracer) {
diff --git a/tests/helpers/test_mkfds.c b/tests/helpers/test_mkfds.c
index 57f99e5a3..ee2d95da8 100644
--- a/tests/helpers/test_mkfds.c
+++ b/tests/helpers/test_mkfds.c
@@ -4413,7 +4413,7 @@ static void list_output_values(const char *factory_name)
 
 static void rename_self(const char *comm)
 {
-	if (prctl(PR_SET_NAME, (unsigned long)comm, 0, 0, 0) < 0)
+	if (prctl(PR_SET_NAME, (unsigned long)comm, 0L, 0L, 0L) < 0)
 		err(EXIT_FAILURE, "failed to rename self via prctl: %s", comm);
 }
 
-- 
2.51.0

Re: [PATCH v2 0/1] Call prctl(2) with signed long integers, and avoid casts

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 10989 bytes --]

Hi,

On Wed, Dec 03, 2025 at 09:50:27PM +0100, Alejandro Colomar wrote:
> Hi!
> 
> Karel reminded me of this old patch.  Here's a revision of the patch.
> Major changes in v2:
> 
> -  Don't specify the 5 arguments unnecessarily.
> 
> BTW, I've developed a header file that might be useful for the general
> public.  See in a reply to this mail.

Here it is.  I think it would be useful to provide this in some libprctl
library so that everyone can use these, instead of raw prctl(2).  What
do you think?  We could start by including this header file within
util-linux, and then consider providing in a separate git repository so
that distros can package it as a system library.


Have a lovely night!
Alex

---
// Copyright 2025, Alejandro Colomar <alx@kernel.org>
// SPDX-License-Identifier: LGPL-3.0-only WITH LGPL-3.0-linking-exception

#include <linux/filter.h>
#include <stddef.h>
#include <stdint.h>
#include <sys/prctl.h>
#include <sys/types.h>

#if __has_include(<seccomp.h>)
# include <seccomp.h>
#endif

#if !defined(__has_c_attribute)
# define __has_c_attribute(x)  0
#endif
#if __has_attribute(__null_terminated_string_arg__)
# define LIBPRCTL_ATTR_STRING(i)  __attribute__((__null_terminated_string_arg__(i)))
#endif
#if __has_attribute(__deprecated__)
# define LIBPRCTL_ATTR_DEPRECATED  __attribute__((__deprecated__))
#endif

_Static_assert(sizeof(long) == sizeof(void *), "");
_Static_assert(sizeof(long) == sizeof(size_t), "");

static inline int pr_cap_ambient_raise(int64_t cap) {
	return prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, (long){cap}, 0L, 0L);
}
static inline int pr_cap_ambient_lower(int64_t cap) {
	return prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_LOWER, (long){cap}, 0L, 0L);
}
static inline int pr_cap_ambient_is_set(int64_t cap) {
	return prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET, (long){cap}, 0L,0L);
}
static inline int pr_cap_ambient_clear_all(void) {
	return prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_CLEAR_ALL, 0L, 0L, 0L);
}
static inline int pr_capbset_read(int64_t cap) {
	return prctl(PR_CAPBSET_READ, (long){cap});
}
static inline int pr_capbset_drop(int64_t cap) {
	return prctl(PR_CAPBSET_DROP, (long){cap});
}
static inline int pr_set_child_subreaper(bool set) {
	return prctl(PR_SET_CHILD_SUBREAPER, (long){set});
}
static inline int pr_get_child_subreaper(bool *isset)
{
	int  ret, x;
	ret = prctl(PR_GET_CHILD_SUBREAPER, &x);
	*isset = x;
	return ret;
}
static inline int pr_set_dumpable(bool set) {
	return prctl(PR_SET_DUMPABLE, (long){set});
}
static inline int pr_get_dumpable(void) {
	return prctl(PR_GET_DUMPABLE);
}
static inline int pr_set_endian(int endianness) {
	return prctl(PR_SET_ENDIAN, (long){endianness});
}
static inline int pr_get_endian(int *endianness) {
	return prctl(PR_GET_ENDIAN, endianness);
}
static inline int pr_set_fp_mode(unsigned int mode) {
	return prctl(PR_SET_FP_MODE, (unsigned long){mode});
}
static inline int pr_get_fp_mode(void) {
	return prctl(PR_GET_FP_MODE);
}
static inline int pr_set_fpemu(int fpemu) {
	return prctl(PR_SET_FPEMU, (long){fpemu});
}
static inline int pr_get_fpemu(int *fpemu) {
	return prctl(PR_GET_FPEMU, fpemu);
}
static inline int pr_set_fpexec(int mode) {
	return prctl(PR_SET_FPEXC, (long){mode});
}
static inline int pr_get_fpexec(int *mode) {
	return prctl(PR_GET_FPEXC, mode);
}
static inline int pr_set_io_flusher(bool set) {
	return prctl(PR_SET_IO_FLUSHER, (long){set}, 0L, 0L, 0L);
}
static inline int pr_get_io_flusher(void) {
	return prctl(PR_GET_IO_FLUSHER, 0L, 0L, 0L, 0L);
}
static inline int pr_set_keepcaps(bool set) {
	return prctl(PR_SET_KEEPCAPS, (long){set});
}
static inline int pr_get_keepcaps(void) {
	return prctl(PR_GET_KEEPCAPS);
}
static inline int pr_mce_kill_clear(void) {
	return prctl(PR_MCE_KILL, PR_MCE_KILL_CLEAR, 0L, 0L, 0L);
}
static inline int pr_mce_kill_set(int policy) {
	return prctl(PR_MCE_KILL, PR_MCE_KILL_SET, (long){policy}, 0L, 0L);
}
static inline int pr_mce_kill_get(void) {
	return prctl(PR_MCE_KILL_GET, 0L, 0L, 0L, 0L);
}
static inline int pr_set_mm_start_code(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_START_CODE, addr, 0L, 0L);
}
static inline int pr_set_mm_end_code(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_END_CODE, addr, 0L, 0L);
}
static inline int pr_set_mm_start_data(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_START_DATA, addr, 0L, 0L);
}
static inline int pr_set_mm_end_data(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_END_DATA, addr, 0L, 0L);
}
static inline int pr_set_mm_start_stack(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_START_STACK, addr, 0L, 0L);
}
static inline int pr_set_mm_start_brk(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_START_BRK, addr, 0L, 0L);
}
static inline int pr_set_mm_brk(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_BRK, addr, 0L, 0L);
}
static inline int pr_set_mm_arg_start(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_ARG_START, addr, 0L, 0L);
}
static inline int pr_set_mm_arg_end(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_ARG_END, addr, 0L, 0L);
}
static inline int pr_set_mm_env_start(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_ENV_START, addr, 0L, 0L);
}
static inline int pr_set_mm_env_end(const void *addr) {
	return prctl(PR_SET_MM, PR_SET_MM_ENV_END, addr, 0L, 0L);
}
static inline int pr_set_mm_auxv(const void *addr, size_t size) {
	return prctl(PR_SET_MM, PR_SET_MM_AUXV, addr, size, 0L);
}
static inline int pr_set_mm_exe_file(int fd) {
	return prctl(PR_SET_MM, PR_SET_MM_EXE_FILE, (long){fd}, 0L, 0L);
}
static inline int pr_set_mm_map(struct prctl_mm_map *map)
{
	return prctl(PR_SET_MM, PR_SET_MM_MAP, map,
	             sizeof(struct prctl_mm_map), 0L);
}
static inline int pr_set_mm_map_size(size_t *size)
{
	int  ret, x;
	ret = prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, &x, 0L, 0L);
	*size = x;
	return ret;
}
LIBPRCTL_ATTR_STRING(3)
static inline int
pr_set_vma_anon_name(const void *addr, size_t size, const char *name)
{
	return prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, addr, size, name);
}
LIBPRCTL_ATTR_DEPRECATED
static inline int pr_mpx_enable_management(void) {
	return prctl(PR_MPX_ENABLE_MANAGEMENT, 0L, 0L, 0L, 0L);
}
LIBPRCTL_ATTR_DEPRECATED
static inline int pr_mpx_disable_management(void) {
	return prctl(PR_MPX_DISABLE_MANAGEMENT, 0L, 0L, 0L, 0L);
}
LIBPRCTL_ATTR_STRING(1)
static inline int pr_set_name(const char *name) {
	return prctl(PR_SET_NAME, name);
}
LIBPRCTL_ATTR_STRING(1)
static inline int pr_get_name(char name[16]) {
	return prctl(PR_GET_NAME, name);
}
static inline int pr_set_no_new_privs(void) {
	return prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, 0L, 0L);
}
static inline int pr_get_no_new_privs(void) {
	return prctl(PR_GET_NO_NEW_PRIVS, 0L, 0L, 0L, 0L);
}
static inline int pr_pac_reset_keys(unsigned long keys) {
	return prctl(PR_PAC_RESET_KEYS, keys, 0L, 0L, 0L);
}
static inline int pr_set_pdeathsig(int sig) {
	return prctl(PR_SET_PDEATHSIG, (long){sig});
}
static inline int pr_get_pdeathsig(int *sig) {
	return prctl(PR_GET_PDEATHSIG, sig);
}
static inline int pr_set_ptracer(pid_t pid) {
	return prctl(PR_SET_PTRACER, (long){pid});
}
#if __has_include(<seccomp.h>)
LIBPRCTL_ATTR_DEPRECATED
static inline int pr_set_seccomp_mode_strict(void) {
	return prctl(PR_SET_SECCOMP, SECCOMP_MODE_STRICT);
}
LIBPRCTL_ATTR_DEPRECATED
static inline int pr_set_seccomp_mode_filter(const struct sock_fprog *filter) {
	return prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, filter);
}
#endif
static inline int pr_get_seccomp(void) {
	return prctl(PR_GET_SECCOMP);
}
static inline int pr_set_securebits(unsigned int securebits) {
	return prctl(PR_SET_SECUREBITS, (unsigned long){securebits});
}
static inline int pr_get_securebits(void) {
	return prctl(PR_GET_SECUREBITS);
}
static inline int pr_get_speculation_ctrl(long misfeature) {
	return prctl(PR_GET_SPECULATION_CTRL, misfeature, 0L, 0L, 0L);
}
static inline int pr_set_speculation_ctrl(long misfeature, int ctrl) {
	return prctl(PR_SET_SPECULATION_CTRL, misfeature, (long){ctrl}, 0L, 0L);
}
static inline int pr_sve_set_vl(unsigned long val) {
	return prctl(PR_SVE_SET_VL, val);
}
static inline int pr_sve_get_vl(void) {
	return prctl(PR_SVE_GET_VL);
}
static inline int pr_sys_dispatch_on(size_t off, size_t size, int8_t *sw)
{
	return prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_ON,
	             off, size, sw);
}
static inline int pr_sys_dispatch_off(void) {
	return prctl(PR_SET_SYSCALL_USER_DISPATCH,PR_SYS_DISPATCH_OFF,0L,0L,0L);
}
static inline int pr_set_tagged_addr_ctrl(int mode) {
	return prctl(PR_SET_TAGGED_ADDR_CTRL, (long){mode}, 0L, 0L, 0L);
}
static inline int pr_get_tagged_addr_ctrl(void) {
	return prctl(PR_GET_TAGGED_ADDR_CTRL, 0L, 0L, 0L, 0L);
}
static inline int pr_task_perf_events_enable(void) {
	return prctl(PR_TASK_PERF_EVENTS_ENABLE);
}
static inline int pr_task_perf_events_disable(void) {
	return prctl(PR_TASK_PERF_EVENTS_DISABLE);
}
static inline int pr_set_thp_disable(bool set, unsigned int flags)
{
	return prctl(PR_SET_THP_DISABLE, (long){set},
	             (unsigned long){flags}, 0L, 0L);
}
static inline int pr_get_thp_disable(void) {
	return prctl(PR_GET_THP_DISABLE, 0L, 0L, 0L, 0L);
}
static inline int pr_get_tid_address(int **addrp) {
	return prctl(PR_GET_TID_ADDRESS, addrp);
}
static inline int pr_set_timerslack(unsigned int slack) {
	return prctl(PR_SET_TIMERSLACK, (unsigned long){slack});
}
static inline int pr_get_timerslack(void) {
	return prctl(PR_GET_TIMERSLACK);
}
static inline int pr_set_timing(int mode) {
	return prctl(PR_SET_TIMING, (long){mode});
}
static inline int pr_get_timing(void) {
	return prctl(PR_GET_TIMING);
}
static inline int pr_set_tsc(int mode) {
	return prctl(PR_SET_TSC, (long){mode});
}
static inline int pr_get_tsc(int *mode) {
	return prctl(PR_GET_TSC, mode);
}
static inline int pr_set_unalign(unsigned int bits) {
	return prctl(PR_SET_UNALIGN, (long){bits});
}
static inline int pr_get_unalign(unsigned int *bits) {
	return prctl(PR_GET_UNALIGN, bits);
}
static inline int pr_get_auxv(void *auxv, size_t size) {
	return prctl(PR_GET_AUXV, auxv, size, 0L, 0L);
}
static inline int pr_set_mdwe(unsigned int mask) {
	return prctl(PR_SET_MDWE, mask, 0L, 0L, 0L);
}
static inline int pr_get_mdwe(void) {
	return prctl(PR_GET_MDWE, 0L, 0L, 0L, 0L);
}
static inline int pr_riscv_set_icache_flush_ctx(int ctx, int scope) {
	return prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX, (long){ctx}, (long){scope});
}
static inline int pr_futex_hash_get_slots(void) {
	return prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_GET_SLOTS);
}
static inline int pr_futex_hash_set_slots(size_t size) {
	return prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS, size, 0L);
}


-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/1] Call prctl(2) with signed long integers, and avoid casts

[Thomas Weißschuh]

Hi!

On 2025-12-03 22:01:18+0100, Alejandro Colomar wrote:
> On Wed, Dec 03, 2025 at 09:50:27PM +0100, Alejandro Colomar wrote:
> > Karel reminded me of this old patch.  Here's a revision of the patch.
> > Major changes in v2:
> > 
> > -  Don't specify the 5 arguments unnecessarily.
> > 
> > BTW, I've developed a header file that might be useful for the general
> > public.  See in a reply to this mail.
> 
> Here it is.  I think it would be useful to provide this in some libprctl
> library so that everyone can use these, instead of raw prctl(2).  What
> do you think?  We could start by including this header file within
> util-linux, and then consider providing in a separate git repository so
> that distros can package it as a system library.

What about fixing raw prctl(2) in libc to avoid the issues you are
fixing in your original patch? prctl() could be a macro which counts its
passed arguments, dispatching to a set of inline functions which then in
turn call the underlying prctl() with the correct set of parameters.
This would be backwards-compatible and safe.

(...)


Thomas

Re: [PATCH v2 0/1] Call prctl(2) with signed long integers, and avoid casts

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]

Hi Thomas!

On Wed, Dec 03, 2025 at 11:12:47PM +0100, Thomas Weißschuh wrote:
> Hi!
> 
> On 2025-12-03 22:01:18+0100, Alejandro Colomar wrote:
> > On Wed, Dec 03, 2025 at 09:50:27PM +0100, Alejandro Colomar wrote:
> > > Karel reminded me of this old patch.  Here's a revision of the patch.
> > > Major changes in v2:
> > > 
> > > -  Don't specify the 5 arguments unnecessarily.
> > > 
> > > BTW, I've developed a header file that might be useful for the general
> > > public.  See in a reply to this mail.
> > 
> > Here it is.  I think it would be useful to provide this in some libprctl
> > library so that everyone can use these, instead of raw prctl(2).  What
> > do you think?  We could start by including this header file within
> > util-linux, and then consider providing in a separate git repository so
> > that distros can package it as a system library.
> 
> What about fixing raw prctl(2) in libc to avoid the issues you are
> fixing in your original patch? prctl() could be a macro which counts its
> passed arguments, dispatching to a set of inline functions which then in
> turn call the underlying prctl() with the correct set of parameters.
> This would be backwards-compatible and safe.

I'm not sure how this would be implementable in a single macro.  If
possible at all, I guess such a macro would be easily over 1k lines of
hardly readable code.  I think providing these inline functions in libc
would be more reasonable.  I can ask glibc and see what they think.


Have a lovely night!
Alex

> 
> (...)
> 
> 
> Thomas

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] Call prctl(2) with long integers, specify 5 arguments, and avoid casts

[Adhemerval Zanella Netto]

On 01/06/24 09:23, Alejandro Colomar wrote:
> Hi Thomas,
> 
> On Sat, Jun 01, 2024 at 01:05:02PM GMT, Thomas Weißschuh wrote:
>> On 2024-06-01 11:31:56+0000, Alejandro Colomar wrote:
>>> Since libc's prctl(2) wrapper is a variadic function, arguments must
>>> have the right width.  Otherwise, the behavior is undefined.
>>
>> Ack.
>>
>>> Also, the 5 arguments must be specified always, or the behavior is also
>>> undefined.  libc reads 5 values and passes them all to the kernel, so if
>>> one is uninitialized, the kernel will receive garbagge, which could
>>> result in EINVAL (most likely), or worse, a different action.
>>
>> This seems surprising.
>>
>> The kernel should only check the arguments it documents and not more.
> 
> Hmmm, some prctl(2) calls don't document a need for passing 0 (probably
> for legacy compatibility; you're right.  Only newer prctl(2)s check
> those args.
> 
> And see for example these kernel commit:
> 
> 	commit e9d1b4f3c60997fe197bf0243cb4a41a44387a88
> 	Author: Dave Hansen <dave.hansen@linux.intel.com>
> 	Date:   Thu Jan 8 14:30:22 2015 -0800
> 
> 	    x86, mpx: Strictly enforce empty prctl() args
> 	    
> 	    Description from Michael Kerrisk.  He suggested an identical patch
> 	    to one I had already coded up and tested.
> 	    
> 	    commit fe3d197f8431 "x86, mpx: On-demand kernel allocation of bounds
> 	    tables" added two new prctl() operations, PR_MPX_ENABLE_MANAGEMENT and
> 	    PR_MPX_DISABLE_MANAGEMENT.  However, no checks were included to ensure
> 	    that unused arguments are zero, as is done in many existing prctl()s
> 	    and as should be done for all new prctl()s. This patch adds the
> 	    required checks.
> 	    
> 	    Suggested-by: Andy Lutomirski <luto@amacapital.net>
> 	    Suggested-by: Michael Kerrisk <mtk.manpages@gmail.com>
> 	    Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> 	    Cc: Dave Hansen <dave@sr71.net>
> 	    Link: http://lkml.kernel.org/r/20150108223022.7F56FD13@viggo.jf.intel.com
> 	    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> 	diff --git a/kernel/sys.c b/kernel/sys.c
> 	index a8c9f5a7dda6..ea9c88109894 100644
> 	--- a/kernel/sys.c
> 	+++ b/kernel/sys.c
> 	@@ -2210,9 +2210,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> 			up_write(&me->mm->mmap_sem);
> 			break;
> 		case PR_MPX_ENABLE_MANAGEMENT:
> 	+               if (arg2 || arg3 || arg4 || arg5)
> 	+                       return -EINVAL;
> 			error = MPX_ENABLE_MANAGEMENT(me);
> 			break;
> 		case PR_MPX_DISABLE_MANAGEMENT:
> 	+               if (arg2 || arg3 || arg4 || arg5)
> 	+                       return -EINVAL;
> 			error = MPX_DISABLE_MANAGEMENT(me);
> 			break;
> 		default:
> 
> And this one too:
> 
> 	commit 3e91ec89f527b9870fe42dcbdb74fd389d123a95
> 	Author: Catalin Marinas <catalin.marinas@arm.com>
> 	Date:   Thu Aug 15 16:44:00 2019 +0100
> 
> 	    arm64: Tighten the PR_{SET, GET}_TAGGED_ADDR_CTRL prctl() unused arguments
> 	    
> 	    Require that arg{3,4,5} of the PR_{SET,GET}_TAGGED_ADDR_CTRL prctl and
> 	    arg2 of the PR_GET_TAGGED_ADDR_CTRL prctl() are zero rather than ignored
> 	    for future extensions.
> 	    
> 	    Acked-by: Andrey Konovalov <andreyknvl@google.com>
> 	    Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> 	    Signed-off-by: Will Deacon <will@kernel.org>
> 
> 	diff --git a/kernel/sys.c b/kernel/sys.c
> 	index c6c4d5358bd3..ec48396b4943 100644
> 	--- a/kernel/sys.c
> 	+++ b/kernel/sys.c
> 	@@ -2499,9 +2499,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> 			error = PAC_RESET_KEYS(me, arg2);
> 			break;
> 		case PR_SET_TAGGED_ADDR_CTRL:
> 	+               if (arg3 || arg4 || arg5)
> 	+                       return -EINVAL;
> 			error = SET_TAGGED_ADDR_CTRL(arg2);
> 			break;
> 		case PR_GET_TAGGED_ADDR_CTRL:
> 	+               if (arg2 || arg3 || arg4 || arg5)
> 	+                       return -EINVAL;
> 			error = GET_TAGGED_ADDR_CTRL();
> 			break;
> 		default:
> 
> In the few calls that util-linux makes without specifying all 5 args,
> the kernel seems to not do the checks (in some old prctl(2)s they didn't
> have that check, and nobody seems to have cared enough to add it), so
> it's more like we're lucky (or unlucky, depending on how you see it).
> 
>> glibc itself doesn't even specify all five arguments in its own calls to
>> prctl().
> 
> glibc itself is wrong.  I'm even surprised that the PR_* macros from the
> kernel UAPI for arg2 work without specifying the L suffix on them, but
> it's probably just luck.
> 
> <https://lore.kernel.org/linux-api/20240528114750.106187-1-alx@kernel.org/T/#u>
> 
>> If all five arguments are really required then prctl() wouldn't need to
>> be variadic.
> 
> Indeed.  I guess that's for historic reasons, rather than actual
> necessity; but I don't know for sure.
> 
>> How is random non-zero data less valid than a essentially random zero?
>> And if the kernel actually validates this, how has it ever worked before?
> 
> They only added validation for (all) new prctl(2) calls, plus maybe some
> old ones, but not all.  In the ones used in util-linux that don't
> specify zero, I've checked now that the kernel doesn't validate.
> 
> However, a call such as
> 
> 	prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)
> 
> (this call exists in util-linux)
> actually means
> 
> 	prctl(PR_SET_NO_NEW_PRIVS, 1L, 0L, random, random)
> 
> and it supposedly has been working so far.  Those random bits are
> probably 0 most of the time, for some reason.  And the kernel does check
> this one:
> 
> 	$ sed -n /PR_SET_NO_NEW_PRIVS/,+2p <kernel/sys.c
> 		case PR_SET_NO_NEW_PRIVS:
> 			if (arg2 != 1 || arg3 || arg4 || arg5)
> 				return -EINVAL;
> 
>> Other popular software like systemd or opendjk also don't specify unused arguments.
> 
> I've also checked that the ones that systemd uses without specifying all
> 5 args, they are not checked by the kernel.
> 
>> So it doesn't really seem "broken".
>> If the patch is more about "being on the safe side", then this should be
>> spelled out.
> 
> Still, libc reads those values (on x32) which results in Undefined
> Behavior inside glibc.  Which is a bad thing.  Not broken, because the
> compiler has little information to exploit that UB, but not a good thing
> either.
> 
> 	$ grepc __prctl .
> 	./include/sys/prctl.h:extern int __prctl (int __option, ...);
> 	./sysdeps/unix/sysv/linux/x86_64/x32/prctl.c:int
> 	__prctl (int option, ...)
> 	{
> 	  va_list arg;
> 	  va_start (arg, option);
> 	  unsigned long int arg2 = va_arg (arg, unsigned long int);
> 	  unsigned long int arg3 = va_arg (arg, unsigned long int);
> 	  unsigned long int arg4 = va_arg (arg, unsigned long int);
> 	  unsigned long int arg5 = va_arg (arg, unsigned long int);
> 	  va_end (arg);
> 	  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
> 	}
> 
> It's arguably less broken than the missing 'L', though.

The x32 and or1k (which also uses similar implementation) does seems broken
without checking the 'option' argument to see which arg we can va_arg.

The problem is adding this logic on libc will add some forward-compatibility
that we try to avoid (newer kernel prctl additions might now work correctly).

I am not sure why we haven't switch x32 back to the assembly wrappers
with 6a04404521ac4119ae36827eeb288ea84eee7cf6 fix (BZ#29770).  H.J, can
use remove the x32 C version (and also or1k as well)?

> 
>> (Plus the cases where documented, required arguments are missing)
> 
> None of the cases where we omit the arguments are checked by the kernel.
> 
> 
> Have a lovely day!
> Alex
> 

[PATCH] x32: Switch back to assembly syscall wrapper for prctl

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On Thu, Dec 4, 2025 at 10:06 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
> The x32 and or1k (which also uses similar implementation) does seems broken
> without checking the 'option' argument to see which arg we can va_arg.
>
> The problem is adding this logic on libc will add some forward-compatibility
> that we try to avoid (newer kernel prctl additions might now work correctly).
>
> I am not sure why we haven't switch x32 back to the assembly wrappers
> with 6a04404521ac4119ae36827eeb288ea84eee7cf6 fix (BZ#29770).  H.J, can
> use remove the x32 C version (and also or1k as well)?

Since the variadic prctl function takes at most 5 integer arguments which
are passed in the same integer registers on x32 as the function with 5
integer arguments, we can safely use assembly syscall wrapper for prctl
for x32.

Tested on x32.  I leave or1k alone since I don't know if it is safe to
do the same.

-- 
H.J.

[-- Attachment #2: 0001-x32-Switch-back-to-assembly-syscall-wrapper-for-prct.patch --]
[-- Type: text/x-patch, Size: 2569 bytes --]

From e6a14154e90e1e6ba2372340b62dae13667bc8ba Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 7 Dec 2025 11:33:33 +0800
Subject: [PATCH] x32: Switch back to assembly syscall wrapper for prctl

Since the variadic prctl function takes at most 5 integer arguments which
are passed in the same integer registers on x32 as the function with 5
integer arguments, we can safely use assembly syscall wrapper for prctl
for x32.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 sysdeps/unix/sysv/linux/x86_64/x32/prctl.c | 42 ----------------------
 1 file changed, 42 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/prctl.c

diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/prctl.c b/sysdeps/unix/sysv/linux/x86_64/x32/prctl.c
deleted file mode 100644
index 714fd28837..0000000000
--- a/sysdeps/unix/sysv/linux/x86_64/x32/prctl.c
+++ /dev/null
@@ -1,42 +0,0 @@
-/* prctl - Linux specific syscall.  x86-64 x32 version.
-   Copyright (C) 2020-2025 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#include <sysdep.h>
-#include <stdarg.h>
-#include <sys/prctl.h>
-
-/* Unconditionally read all potential arguments.  This may pass
-   garbage values to the kernel, but avoids the need for teaching
-   glibc the argument counts of individual options (including ones
-   that are added to the kernel in the future).  */
-
-int
-__prctl (int option, ...)
-{
-  va_list arg;
-  va_start (arg, option);
-  unsigned long int arg2 = va_arg (arg, unsigned long int);
-  unsigned long int arg3 = va_arg (arg, unsigned long int);
-  unsigned long int arg4 = va_arg (arg, unsigned long int);
-  unsigned long int arg5 = va_arg (arg, unsigned long int);
-  va_end (arg);
-  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
-}
-
-libc_hidden_def (__prctl)
-weak_alias (__prctl, prctl)
-- 
2.52.0

Re: [PATCH] x32: Switch back to assembly syscall wrapper for prctl

[Florian Weimer]

* H. J. Lu:

> On Thu, Dec 4, 2025 at 10:06 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>
>> The x32 and or1k (which also uses similar implementation) does seems broken
>> without checking the 'option' argument to see which arg we can va_arg.
>>
>> The problem is adding this logic on libc will add some forward-compatibility
>> that we try to avoid (newer kernel prctl additions might now work correctly).
>>
>> I am not sure why we haven't switch x32 back to the assembly wrappers
>> with 6a04404521ac4119ae36827eeb288ea84eee7cf6 fix (BZ#29770).  H.J, can
>> use remove the x32 C version (and also or1k as well)?
>
> Since the variadic prctl function takes at most 5 integer arguments which
> are passed in the same integer registers on x32 as the function with 5
> integer arguments, we can safely use assembly syscall wrapper for prctl
> for x32.

The C implementation clears the upper 32 bits of registers.  Does the
assembler wrapper do the same?

Thanks,
Florian

Re: [PATCH] x32: Switch back to assembly syscall wrapper for prctl

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

Hi H.J.,

On Sun, Dec 07, 2025 at 11:52:08AM +0800, H.J. Lu wrote:
> On Thu, Dec 4, 2025 at 10:06 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
> >
> > The x32 and or1k (which also uses similar implementation) does seems broken
> > without checking the 'option' argument to see which arg we can va_arg.
> >
> > The problem is adding this logic on libc will add some forward-compatibility
> > that we try to avoid (newer kernel prctl additions might now work correctly).
> >
> > I am not sure why we haven't switch x32 back to the assembly wrappers
> > with 6a04404521ac4119ae36827eeb288ea84eee7cf6 fix (BZ#29770).  H.J, can
> > use remove the x32 C version (and also or1k as well)?
> 
> Since the variadic prctl function takes at most 5 integer arguments which
> are passed in the same integer registers on x32 as the function with 5
> integer arguments, we can safely use assembly syscall wrapper for prctl
> for x32.
> 
> Tested on x32.  I leave or1k alone since I don't know if it is safe to
> do the same.

Thanks!  Would you mind adding the following tag?

	Reported-by: Alejandro Colomar <alx@kernel.org>

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2] x32: Implement prctl in assembly

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

On Sun, Dec 7, 2025 at 5:41 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Thu, Dec 4, 2025 at 10:06 PM Adhemerval Zanella Netto
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >> The x32 and or1k (which also uses similar implementation) does seems broken
> >> without checking the 'option' argument to see which arg we can va_arg.
> >>
> >> The problem is adding this logic on libc will add some forward-compatibility
> >> that we try to avoid (newer kernel prctl additions might now work correctly).
> >>
> >> I am not sure why we haven't switch x32 back to the assembly wrappers
> >> with 6a04404521ac4119ae36827eeb288ea84eee7cf6 fix (BZ#29770).  H.J, can
> >> use remove the x32 C version (and also or1k as well)?
> >
> > Since the variadic prctl function takes at most 5 integer arguments which
> > are passed in the same integer registers on x32 as the function with 5
> > integer arguments, we can safely use assembly syscall wrapper for prctl
> > for x32.
>
> The C implementation clears the upper 32 bits of registers.  Does the
> assembler wrapper do the same?
>

Here is the v2 patch to implement prctl in assembly for x32.

Since the variadic prctl function takes at most 5 integer arguments which
are passed in the same integer registers on x32 as the function with 5
integer arguments, we can use assembly for prctl.  Since upper 32-bits in
the last 4 arguments of prctl must be cleared to match the x32 prctl
syscall interface where the last 4 arguments are unsigned 64 bit longs,
implement prctl in assembly to clear upper 32-bits in the last 4 arguments
and add a test to verify it.


-- 
H.J.

[-- Attachment #2: v2-0001-x32-Implement-prctl-in-assembly.patch --]
[-- Type: text/x-patch, Size: 5782 bytes --]

From 20f922113e65f4806db724935f7a747db639db42 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 7 Dec 2025 11:33:33 +0800
Subject: [PATCH v2] x32: Implement prctl in assembly

Since the variadic prctl function takes at most 5 integer arguments which
are passed in the same integer registers on x32 as the function with 5
integer arguments, we can use assembly for prctl.  Since upper 32-bits in
the last 4 arguments of pcrtl must be cleared to match the x32 prctl
syscall interface where the last 4 arguments are unsigned 64 bit longs,
implement prctl in assembly to clear upper 32-bits in the last 4 arguments
and add a test to verify it.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 sysdeps/unix/sysv/linux/x86_64/x32/Makefile   |  6 ++
 .../linux/x86_64/x32/{prctl.c => prctl.S}     | 38 ++++++-----
 .../sysv/linux/x86_64/x32/tst-prctl-x32.c     | 63 +++++++++++++++++++
 3 files changed, 87 insertions(+), 20 deletions(-)
 rename sysdeps/unix/sysv/linux/x86_64/x32/{prctl.c => prctl.S} (50%)
 create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/tst-prctl-x32.c

diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
index 16b768d8ba..004f449883 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
@@ -3,6 +3,12 @@ default-abi := x32
 
 ifeq ($(subdir),misc)
 sysdep_routines += arch_prctl
+
+tests += \
+  tst-prctl-x32 \
+# tests
+
+CFLAGS-tst-prctl-x32.c += $(no-stack-protector)
 endif
 
 ifeq ($(subdir),conform)
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/prctl.c b/sysdeps/unix/sysv/linux/x86_64/x32/prctl.S
similarity index 50%
rename from sysdeps/unix/sysv/linux/x86_64/x32/prctl.c
rename to sysdeps/unix/sysv/linux/x86_64/x32/prctl.S
index 714fd28837..41f643ea80 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/prctl.c
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/prctl.S
@@ -1,5 +1,5 @@
-/* prctl - Linux specific syscall.  x86-64 x32 version.
-   Copyright (C) 2020-2025 Free Software Foundation, Inc.
+/* The prctl system call.  Linux/x32 version.
+   Copyright (C) 2025 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -17,26 +17,24 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
-#include <stdarg.h>
-#include <sys/prctl.h>
 
-/* Unconditionally read all potential arguments.  This may pass
-   garbage values to the kernel, but avoids the need for teaching
-   glibc the argument counts of individual options (including ones
-   that are added to the kernel in the future).  */
+/* Clear upper 32-bits in the last 4 arguments.  Since the first argument
+   of prctl is int, leave it alone.  */
+#undef	DO_CALL
+#define DO_CALL(syscall_name, args, ulong_arg_1, ulong_arg_2) \
+  movl %esi, %esi;			\
+  movl %edx, %edx;			\
+  movl %ecx, %r10d;			\
+  movl %r8d, %r8d;			\
+  movl $SYS_ify (syscall_name), %eax;	\
+  syscall;
 
-int
-__prctl (int option, ...)
-{
-  va_list arg;
-  va_start (arg, option);
-  unsigned long int arg2 = va_arg (arg, unsigned long int);
-  unsigned long int arg3 = va_arg (arg, unsigned long int);
-  unsigned long int arg4 = va_arg (arg, unsigned long int);
-  unsigned long int arg5 = va_arg (arg, unsigned long int);
-  va_end (arg);
-  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
-}
+PSEUDO (__prctl, prctl, 5)
+	ret
+PSEUDO_END (__prctl)
 
 libc_hidden_def (__prctl)
 weak_alias (__prctl, prctl)
+hidden_weak (prctl)
+weak_alias (__prctl, __prctl_time64)
+hidden_weak (__prctl_time64)
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/tst-prctl-x32.c b/sysdeps/unix/sysv/linux/x86_64/x32/tst-prctl-x32.c
new file mode 100644
index 0000000000..295b09e364
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/tst-prctl-x32.c
@@ -0,0 +1,63 @@
+/* Smoke test for prctl.
+   Copyright (C) 2021-2025 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+#include <sys/prctl.h>
+#include <support/check.h>
+
+/* On x32, when parameters are passed in 64-bit registers, only the lower
+   32 bits are used and the upper 32 bits must be cleared.  */
+typedef union
+{
+  struct
+    {
+      union
+	{
+	  const char *ptr;
+	  int i1;
+	};
+      int i2;
+    } s;
+  long long ll;
+} parameter_t;
+
+static int
+__attribute__ ((noipa))
+do_prctl (int op, long long arg1, long long arg2, long long arg3,
+	  long long arg4)
+{
+  return prctl (op, arg1, arg2, arg3, arg4);
+}
+
+static int
+do_test (void)
+{
+  parameter_t name = { { { "thread name" }, -1 } };
+  parameter_t zero = { { { 0 }, -2 } };
+  TEST_COMPARE (do_prctl (PR_SET_NAME, name.ll, zero.ll, zero.ll,
+			  zero.ll), 0);
+  char buffer[16] = { 0, };
+  name.s.ptr = buffer;
+  TEST_COMPARE (do_prctl (PR_GET_NAME, name.ll, zero.ll, zero.ll,
+			  zero.ll), 0);
+  char expected[16] = "thread name";
+  TEST_COMPARE_BLOB (buffer, sizeof (buffer), expected, sizeof (expected));
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.52.0

Re: [PATCH v2] x32: Implement prctl in assembly

[Florian Weimer]

* H. J. Lu:
> Here is the v2 patch to implement prctl in assembly for x32.
>
> Since the variadic prctl function takes at most 5 integer arguments which
> are passed in the same integer registers on x32 as the function with 5
> integer arguments, we can use assembly for prctl.  Since upper 32-bits in
> the last 4 arguments of prctl must be cleared to match the x32 prctl
> syscall interface where the last 4 arguments are unsigned 64 bit longs,
> implement prctl in assembly to clear upper 32-bits in the last 4 arguments
> and add a test to verify it.

What's the advantage of the assembler implementation over the C
implementation?  I'm missing the context for this change.

Thanks,
Florian

Re: [PATCH v2] x32: Implement prctl in assembly

[H.J. Lu]

On Mon, Dec 8, 2025 at 4:11 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
> > Here is the v2 patch to implement prctl in assembly for x32.
> >
> > Since the variadic prctl function takes at most 5 integer arguments which
> > are passed in the same integer registers on x32 as the function with 5
> > integer arguments, we can use assembly for prctl.  Since upper 32-bits in
> > the last 4 arguments of prctl must be cleared to match the x32 prctl
> > syscall interface where the last 4 arguments are unsigned 64 bit longs,
> > implement prctl in assembly to clear upper 32-bits in the last 4 arguments
> > and add a test to verify it.
>
> What's the advantage of the assembler implementation over the C
> implementation?  I'm missing the context for this change.
>

It is inspired by

commit 6a04404521ac4119ae36827eeb288ea84eee7cf6
Author: Florian Weimer <fweimer@redhat.com>
Date:   Sat Feb 17 09:17:04 2024 +0100

    Linux: Switch back to assembly syscall wrapper for prctl (bug 29770)

The difference is

00000000 <__GI___prctl>:
   0: f3 0f 1e fa          endbr64
   4: 8d 44 24 08          lea    0x8(%rsp),%eax
   8: 48 89 74 24 d0        mov    %rsi,-0x30(%rsp)
   d: 48 63 ff              movslq %edi,%rdi
  10: 8b 74 24 d0          mov    -0x30(%rsp),%esi
  14: 89 44 24 c0          mov    %eax,-0x40(%rsp)
  18: 8d 44 24 c8          lea    -0x38(%rsp),%eax
  1c: 48 89 54 24 d8        mov    %rdx,-0x28(%rsp)
  21: 8b 54 24 d8          mov    -0x28(%rsp),%edx
  25: 48 89 4c 24 e0        mov    %rcx,-0x20(%rsp)
  2a: 44 8b 54 24 e0        mov    -0x20(%rsp),%r10d
  2f: 4c 89 44 24 e8        mov    %r8,-0x18(%rsp)
  34: 44 8b 44 24 e8        mov    -0x18(%rsp),%r8d
  39: 89 44 24 c4          mov    %eax,-0x3c(%rsp)
  3d: b8 9d 00 00 40        mov    $0x4000009d,%eax
  42: c7 44 24 b8 08 00 00 00 movl   $0x8,-0x48(%rsp)
  4a: 0f 05                syscall
  4c: 3d 00 f0 ff ff        cmp    $0xfffff000,%eax
  51: 77 05                ja     58 <__GI___prctl+0x58>
  53: c3                    ret
  54: 0f 1f 40 00          nopl   0x0(%rax)
  58: f7 d8                neg    %eax
  5a: 64 8b 14 25 00 00 00 00 mov    %fs:0x0,%edx
  62: 40 03 15 00 00 00 00 rex add 0x0(%rip),%edx        # 69
<__GI___prctl+0x69>
  69: 67 89 02              mov    %eax,(%edx)
  6c: b8 ff ff ff ff        mov    $0xffffffff,%eax
  71: c3                    ret

vs

00000000 <__GI___prctl>:
   0: 89 f6                mov    %esi,%esi
   2: 89 d2                mov    %edx,%edx
   4: 41 89 ca              mov    %ecx,%r10d
   7: 45 89 c0              mov    %r8d,%r8d
   a: b8 9d 00 00 40        mov    $0x4000009d,%eax
   f: 0f 05                syscall
  11: 48 3d 01 f0 ff ff    cmp    $0xfffffffffffff001,%rax
  17: 73 01                jae    1a <__GI___prctl+0x1a>
  19: c3                    ret
  1a: 48 8b 0d 00 00 00 00 mov    0x0(%rip),%rcx        # 21 <__GI___prctl+0x21>
  21: f7 d8                neg    %eax
  23: 64 89 01              mov    %eax,%fs:(%rcx)
  26: 83 c8 ff              or     $0xffffffff,%eax
  29: c3                    ret


-- 
H.J.

Re: [PATCH v2] x32: Implement prctl in assembly

[Florian Weimer]

* H. J. Lu:

> On Mon, Dec 8, 2025 at 4:11 PM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>> > Here is the v2 patch to implement prctl in assembly for x32.
>> >
>> > Since the variadic prctl function takes at most 5 integer arguments which
>> > are passed in the same integer registers on x32 as the function with 5
>> > integer arguments, we can use assembly for prctl.  Since upper 32-bits in
>> > the last 4 arguments of prctl must be cleared to match the x32 prctl
>> > syscall interface where the last 4 arguments are unsigned 64 bit longs,
>> > implement prctl in assembly to clear upper 32-bits in the last 4 arguments
>> > and add a test to verify it.
>>
>> What's the advantage of the assembler implementation over the C
>> implementation?  I'm missing the context for this change.
>>
>
> It is inspired by
>
> commit 6a04404521ac4119ae36827eeb288ea84eee7cf6
> Author: Florian Weimer <fweimer@redhat.com>
> Date:   Sat Feb 17 09:17:04 2024 +0100
>
>     Linux: Switch back to assembly syscall wrapper for prctl (bug 29770)

The justification for that does not apply to x32, though, because prctl
doesn't take floating point arguments.  I don't have a strong opinion,
the C and assembler versions are of similar complexity.

Thanks,
Florian

Re: [PATCH v2] x32: Implement prctl in assembly

[Adhemerval Zanella Netto]

On 08/12/25 06:09, Florian Weimer wrote:
> * H. J. Lu:
> 
>> On Mon, Dec 8, 2025 at 4:11 PM Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>> * H. J. Lu:
>>>> Here is the v2 patch to implement prctl in assembly for x32.
>>>>
>>>> Since the variadic prctl function takes at most 5 integer arguments which
>>>> are passed in the same integer registers on x32 as the function with 5
>>>> integer arguments, we can use assembly for prctl.  Since upper 32-bits in
>>>> the last 4 arguments of prctl must be cleared to match the x32 prctl
>>>> syscall interface where the last 4 arguments are unsigned 64 bit longs,
>>>> implement prctl in assembly to clear upper 32-bits in the last 4 arguments
>>>> and add a test to verify it.
>>>
>>> What's the advantage of the assembler implementation over the C
>>> implementation?  I'm missing the context for this change.
>>>
>>
>> It is inspired by
>>
>> commit 6a04404521ac4119ae36827eeb288ea84eee7cf6
>> Author: Florian Weimer <fweimer@redhat.com>
>> Date:   Sat Feb 17 09:17:04 2024 +0100
>>
>>     Linux: Switch back to assembly syscall wrapper for prctl (bug 29770)
> 
> The justification for that does not apply to x32, though, because prctl
> doesn't take floating point arguments.  I don't have a strong opinion,
> the C and assembler versions are of similar complexity.

The main justification is UB to va_args *all* the arguments without taking
in the consideration which option is passed.  If x32 requires additional
argument handling to clear the upper 32-bits, there is no advantage of
using the assembly wrapper.

Re: [PATCH v2] x32: Implement prctl in assembly

[Florian Weimer]

* Adhemerval Zanella Netto:

> On 08/12/25 06:09, Florian Weimer wrote:
>> * H. J. Lu:
>> 
>>> On Mon, Dec 8, 2025 at 4:11 PM Florian Weimer <fweimer@redhat.com> wrote:
>>>>
>>>> * H. J. Lu:
>>>>> Here is the v2 patch to implement prctl in assembly for x32.
>>>>>
>>>>> Since the variadic prctl function takes at most 5 integer arguments which
>>>>> are passed in the same integer registers on x32 as the function with 5
>>>>> integer arguments, we can use assembly for prctl.  Since upper 32-bits in
>>>>> the last 4 arguments of prctl must be cleared to match the x32 prctl
>>>>> syscall interface where the last 4 arguments are unsigned 64 bit longs,
>>>>> implement prctl in assembly to clear upper 32-bits in the last 4 arguments
>>>>> and add a test to verify it.
>>>>
>>>> What's the advantage of the assembler implementation over the C
>>>> implementation?  I'm missing the context for this change.
>>>>
>>>
>>> It is inspired by
>>>
>>> commit 6a04404521ac4119ae36827eeb288ea84eee7cf6
>>> Author: Florian Weimer <fweimer@redhat.com>
>>> Date:   Sat Feb 17 09:17:04 2024 +0100
>>>
>>>     Linux: Switch back to assembly syscall wrapper for prctl (bug 29770)
>> 
>> The justification for that does not apply to x32, though, because prctl
>> doesn't take floating point arguments.  I don't have a strong opinion,
>> the C and assembler versions are of similar complexity.
>
> The main justification is UB to va_args *all* the arguments without taking
> in the consideration which option is passed.  If x32 requires additional
> argument handling to clear the upper 32-bits, there is no advantage of
> using the assembly wrapper.

I'm okay with making this change to avoid UB.

Patch looks okay to me.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Minor nit:

+weak_alias (__prctl, __prctl_time64)
+hidden_weak (__prctl_time64)

This isn't necessary because there is no __prctl_time64 on x32.

Thanks,
Florian

[PATCH v3] x32: Implement prctl in assembly

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]

On Mon, Dec 8, 2025 at 10:25 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Adhemerval Zanella Netto:
>
> > On 08/12/25 06:09, Florian Weimer wrote:
> >> * H. J. Lu:
> >>
> >>> On Mon, Dec 8, 2025 at 4:11 PM Florian Weimer <fweimer@redhat.com> wrote:
> >>>>
> >>>> * H. J. Lu:
> >>>>> Here is the v2 patch to implement prctl in assembly for x32.
> >>>>>
> >>>>> Since the variadic prctl function takes at most 5 integer arguments which
> >>>>> are passed in the same integer registers on x32 as the function with 5
> >>>>> integer arguments, we can use assembly for prctl.  Since upper 32-bits in
> >>>>> the last 4 arguments of prctl must be cleared to match the x32 prctl
> >>>>> syscall interface where the last 4 arguments are unsigned 64 bit longs,
> >>>>> implement prctl in assembly to clear upper 32-bits in the last 4 arguments
> >>>>> and add a test to verify it.
> >>>>
> >>>> What's the advantage of the assembler implementation over the C
> >>>> implementation?  I'm missing the context for this change.
> >>>>
> >>>
> >>> It is inspired by
> >>>
> >>> commit 6a04404521ac4119ae36827eeb288ea84eee7cf6
> >>> Author: Florian Weimer <fweimer@redhat.com>
> >>> Date:   Sat Feb 17 09:17:04 2024 +0100
> >>>
> >>>     Linux: Switch back to assembly syscall wrapper for prctl (bug 29770)
> >>
> >> The justification for that does not apply to x32, though, because prctl
> >> doesn't take floating point arguments.  I don't have a strong opinion,
> >> the C and assembler versions are of similar complexity.
> >
> > The main justification is UB to va_args *all* the arguments without taking
> > in the consideration which option is passed.  If x32 requires additional
> > argument handling to clear the upper 32-bits, there is no advantage of
> > using the assembly wrapper.
>
> I'm okay with making this change to avoid UB.
>
> Patch looks okay to me.
>
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
>
> Minor nit:
>
> +weak_alias (__prctl, __prctl_time64)
> +hidden_weak (__prctl_time64)
>
> This isn't necessary because there is no __prctl_time64 on x32.

Fixed in the v3 patch I am checking in.

Thanks.

-- 
H.J.

[-- Attachment #2: v3-0001-x32-Implement-prctl-in-assembly.patch --]
[-- Type: application/x-patch, Size: 5742 bytes --]