linux-kernel - Re: [PATCH 1/5] s390/uvdevice: Add info IOCTL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZF42V9fJFtRLR1Ua@osiris>
Date:   Fri, 12 May 2023 14:51:35 +0200
From:   Heiko Carstens <hca@...ux.ibm.com>
To:     Steffen Eiden <seiden@...ux.ibm.com>
Cc:     kvm@...r.kernel.org, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Viktor Mihajlovski <mihajlov@...ux.ibm.com>,
        Janosch Frank <frankja@...ux.ibm.com>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>,
        Nico Boehr <nrb@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Hendrik Brueckner <brueckner@...ux.ibm.com>
Subject: Re: [PATCH 1/5] s390/uvdevice: Add info IOCTL

On Fri, May 12, 2023 at 11:31:49AM +0200, Steffen Eiden wrote:
> Add an IOCTL that allows userspace to find out which IOCTLs the uvdevice
> supports without trial and error.
> 
> Explicitly expose the IOCTL nr for the request types.
> 
> Signed-off-by: Steffen Eiden <seiden@...ux.ibm.com>
> ---
>  arch/s390/include/uapi/asm/uvdevice.h | 45 +++++++++++++++-
>  drivers/s390/char/uvdevice.c          | 77 ++++++++++++++++++++++++---
>  2 files changed, 114 insertions(+), 8 deletions(-)
...
> +static void __init set_supp_uv_cmds(struct uvio_uvdev_info *info)
> +{
> +	int i;
> +
> +	for (i = 0; i < UVIO_IOCTL_NUM_IOCTLS; i++) {
> +		if (ioctl_nr_to_uvc_bit[i] == -1UL)
> +			continue;
> +		if (!test_bit_inv(ioctl_nr_to_uvc_bit[i], uv_info.inst_calls_list))
> +			continue;
> +		set_bit(i, (unsigned long *)&info->supp_uv_cmds);

Just a minor thing: please use

		__set_bit(i, (unsigned long *)&info->supp_uv_cmds);

set_bit() is the atomic variant which may even generate a compare and swap
loop (dependent on config options) to set bits, which is not what is needed
here. Didn't see that in previous version.
So if atomicity is not needed, please use __set_bit() instead.