Re: [PATCH 3/3] s390/diag: Add diag204 IOCTL to diag misc device

[Mete Durlu <meted@linux.ibm.com>]

On 21/10/2026 16:01, Hendrik Brueckner wrote:
> On Wed, Oct 21, 2026 at 12:28:10PM +0200, Mete Durlu wrote:
>> Add IOCTL implementation necessary to fetch diag204 data from the new
>> diag misc device under /dev.
>>
>> Signed-off-by: Mete Durlu <meted@linux.ibm.com>
>> ---
>>   arch/s390/include/uapi/asm/diag.h  |  8 +++++++
>>   arch/s390/kernel/diag/diag204.c    | 43 ++++++++++++++++++++++++++++++++++++++
>>   arch/s390/kernel/diag/diag_ioctl.h |  3 +++
>>   arch/s390/kernel/diag/diag_misc.c  | 10 +++++++++
>>   4 files changed, 64 insertions(+)
> 
> [...]
> 
>> index efffe02ea02e..ac263baf8d08 100644
>> --- a/arch/s390/kernel/diag/diag_misc.c
>> +++ b/arch/s390/kernel/diag/diag_misc.c
>> @@ -35,6 +35,16 @@ static long diag_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>>   	case DIAG310_GET_MEMTOPBUF:
>>   		rc = diag310_memtop_buf(arg);
>>   		break;
>> +	case DIAG204_GET_LEN:
>> +		rc = -EPERM;
>> +		if (capable(CAP_SYS_ADMIN))
>> +			rc = diag204_len(arg);
>> +		break;
>> +	case DIAG204_GET_BUF:
>> +		rc = -EPERM;
>> +		if (capable(CAP_SYS_ADMIN))
> 
> Why do you restrict this to sys_admin cap? I think that access control could
> be better managed through mode and ownership of the /dev/diag device.

I don't know about other diag codes and I wanted to just preserve the
current authorization level for diag204. If you think the /dev/diag
ownership is a better solution, I wouldn't go against it.

> Assuming that I like to run a service collecting information from diag204
> would require to run with sys_admin capability (almost root authority).
> 
> On a /dev/diag, allowing root and group to access the device would allow
> a service to run with regular and respective group privileges to access it
> (w/o cap sys admin).
> 
> We should revisit the device .mode = 0444 setting and prefer using
> 0440 to prohibit other users and restricting access to root/group ownership
> only.
> 
Sounds like a separate one liner patch involving just
arch/s390/kernel/diag/diag_misc.c