From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: Cast atomic_load_int(9) to signed int when loading `securelevel'
To: Paul Fertser <fercerpav@gmail.com>
Cc: OpenBSD Tech <tech@openbsd.org>
Date: Mon, 18 Nov 2024 00:38:45 +0300

Thanks! Will commit this diff today later.

> On 17 Nov 2024, at 01:58, Paul Fertser <fercerpav@gmail.com> wrote:
> 
> The return value of atomic_load_int(9) is unsigned so needs a cast,
> otherwise securelevel=-1 gets misrepresented.
> 
> ---
> 
> diff --git sys/arch/alpha/alpha/mem.c sys/arch/alpha/alpha/mem.c
> index 88b722f2d2fc..cb448c39c79d 100644
> --- sys/arch/alpha/alpha/mem.c
> +++ sys/arch/alpha/alpha/mem.c
> @@ -77,7 +77,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/amd64/amd64/mem.c sys/arch/amd64/amd64/mem.c
> index 53db257d2fab..ebbef068de7e 100644
> --- sys/arch/amd64/amd64/mem.c
> +++ sys/arch/amd64/amd64/mem.c
> @@ -85,7 +85,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/arm/arm/mem.c sys/arch/arm/arm/mem.c
> index 25f702810ae9..d7a7eef07ecc 100644
> --- sys/arch/arm/arm/mem.c
> +++ sys/arch/arm/arm/mem.c
> @@ -104,7 +104,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/arm64/arm64/mem.c sys/arch/arm64/arm64/mem.c
> index fd0308065a77..c336cf63b9df 100644
> --- sys/arch/arm64/arm64/mem.c
> +++ sys/arch/arm64/arm64/mem.c
> @@ -108,7 +108,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/hppa/hppa/mem.c sys/arch/hppa/hppa/mem.c
> index 2fc070e76c4c..6e32fe9bdaab 100644
> --- sys/arch/hppa/hppa/mem.c
> +++ sys/arch/hppa/hppa/mem.c
> @@ -307,7 +307,7 @@ mmopen(dev_t dev, int flag, int ioflag, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/i386/i386/mem.c sys/arch/i386/i386/mem.c
> index 6d5349492406..174047728306 100644
> --- sys/arch/i386/i386/mem.c
> +++ sys/arch/i386/i386/mem.c
> @@ -79,7 +79,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/m88k/m88k/mem.c sys/arch/m88k/m88k/mem.c
> index 0157fd2a0e13..4e289fa250ad 100644
> --- sys/arch/m88k/m88k/mem.c
> +++ sys/arch/m88k/m88k/mem.c
> @@ -65,7 +65,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/macppc/macppc/mem.c sys/arch/macppc/macppc/mem.c
> index 7e5ce6d1b311..3bf4670c9918 100644
> --- sys/arch/macppc/macppc/mem.c
> +++ sys/arch/macppc/macppc/mem.c
> @@ -197,7 +197,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/mips64/mips64/mem.c sys/arch/mips64/mips64/mem.c
> index 47d49ef0d783..570f0906a447 100644
> --- sys/arch/mips64/mips64/mem.c
> +++ sys/arch/mips64/mips64/mem.c
> @@ -78,7 +78,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/powerpc64/powerpc64/mem.c sys/arch/powerpc64/powerpc64/mem.c
> index 3174f3f61c0d..1d88d1639147 100644
> --- sys/arch/powerpc64/powerpc64/mem.c
> +++ sys/arch/powerpc64/powerpc64/mem.c
> @@ -69,7 +69,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/riscv64/riscv64/mem.c sys/arch/riscv64/riscv64/mem.c
> index cc6d9939dd55..c0cb7138e330 100644
> --- sys/arch/riscv64/riscv64/mem.c
> +++ sys/arch/riscv64/riscv64/mem.c
> @@ -104,7 +104,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/sh/sh/mem.c sys/arch/sh/sh/mem.c
> index 88c156950341..c69aba622bc7 100644
> --- sys/arch/sh/sh/mem.c
> +++ sys/arch/sh/sh/mem.c
> @@ -108,7 +108,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/arch/sparc64/sparc64/mem.c sys/arch/sparc64/sparc64/mem.c
> index 19b8152295d8..8fb33f75e16b 100644
> --- sys/arch/sparc64/sparc64/mem.c
> +++ sys/arch/sparc64/sparc64/mem.c
> @@ -69,7 +69,7 @@ mmopen(dev_t dev, int flag, int mode, struct proc *p)
> 	switch (minor(dev)) {
> 	case 0:
> 	case 1:
> -		if (atomic_load_int(&securelevel) <= 0 ||
> +		if ((int)atomic_load_int(&securelevel) <= 0 ||
> 		    atomic_load_int(&allowkmem))
> 			break;
> 		return (EPERM);
> diff --git sys/kern/kern_sysctl.c sys/kern/kern_sysctl.c
> index b45b2b7ca1dc..69fb8fde0845 100644
> --- sys/kern/kern_sysctl.c
> +++ sys/kern/kern_sysctl.c
> @@ -1173,7 +1173,7 @@ int
> sysctl_securelevel_int(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
>     int *valp)
> {
> -	if (atomic_load_int(&securelevel) > 0)
> +	if ((int)atomic_load_int(&securelevel) > 0)
> 		return (sysctl_rdint(oldp, oldlenp, newp, *valp));
> 	return (sysctl_int(oldp, oldlenp, newp, newlen, valp));
> }
>