linux-kernel - Re: [PATCH net-next v4 5/8] mfd: zl3073x: Add functions to work with register mailboxes

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: <bd645425-b9cb-454d-8971-646501704697@redhat.com>
Date: Thu, 24 Apr 2025 19:10:29 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: netdev@...r.kernel.org, Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
 Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
 Jiri Pirko <jiri@...nulli.us>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Prathosh Satish <Prathosh.Satish@...rochip.com>,
 Lee Jones <lee@...nel.org>, Kees Cook <kees@...nel.org>,
 Andy Shevchenko <andy@...nel.org>, Andrew Morton
 <akpm@...ux-foundation.org>, Michal Schmidt <mschmidt@...hat.com>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org
Subject: Re: [PATCH net-next v4 5/8] mfd: zl3073x: Add functions to work with
 register mailboxes

On 24. 04. 25 6:43 odp., Andrew Lunn wrote:
>> +static int
>> +zl3073x_write_reg(struct zl3073x_dev *zldev, unsigned int reg, const void *val)
>> +{
>> +	unsigned int len;
>> +	u8 buf[6];
>> +	int rc;
>> +
>> +	/* Offset of the last item in the indexed register or offset of
>> +	 * the non-indexed register itself.
>> +	 */
>> +	if (ZL_REG_OFFSET(reg) > ZL_REG_MAX_OFFSET(reg)) {
>> +		dev_err(zldev->dev, "Index of out range for reg 0x%04lx\n",
>> +			ZL_REG_ADDR(reg));
>> +		return -EINVAL;
>> +	}
>> +
>> +	len = ZL_REG_SIZE(reg);
> 
> I suggested you add helpers for zl3073x_write_reg_u8(),
> zl3073x_write_reg_u16(), zl3073x_write_reg_32(), and
> zl3073x_write_reg_48(). The compiler will then do type checking for
> val, ensure what you pass is actually big enough.
> 
> Here you have a void *val. You have no idea how big a value that
> pointer points to, and the compiler is not helping you.

During taking 613cbb91e9ce ("media: Add MIPI CCI register access helper 
functions") approach I found they are using for these functions u64
regardless of register size... Just to accommodate the biggest
possible value. I know about weakness of 'void *' usage but u64 is not
also ideal as the caller is forced to pass always 8 bytes for reading
and forced to reserve 8 bytes for each read value on stack.

> I suggest you add the individual helpers. If you decided to keep the
> register meta data, you can validate the correct helper has been
> called.

Yes, this should be easily implemented, will follow this.

Anyway, still don't know what to do with mailboxes (aka multiple atomic 
register operations). Lee seems to be against the placement of this
code in MFD parent driver.

Each sequence has to be protected by some lock and this lock needs to be
placed in MFD. Yes the routines for MB access can be for example in DPLL
driver but still the locks have to be inside MFD. So they have to be
exposed to sub-devices.

Thanks,
Ivan