netdev - Re: [PATCH v2 07/14] mfd: zl3073x: Add components versions register defs

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 for Android: free password hash cracker in your pocket

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

Message-ID: <098b0477-3367-4f96-906b-520fcd95befb@lunn.ch>
Date: Thu, 10 Apr 2025 19:41:58 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Ivan Vecera <ivecera@...hat.com>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, 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 v2 07/14] mfd: zl3073x: Add components versions register
 defs

> > > +	/* Take device lock */
> > 
> > What is a device lock? Why do you need to comment standard guards/mutexes?
> 
> Just to inform code reader, this is a section that accesses device registers
> that are protected by this zl3073x device lock.

I didn't see a reply to my question about the big picture. Why is the
regmap lock not sufficient. Why do you need a device lock.

> 
> > > +	scoped_guard(zl3073x, zldev) {
> > > +		rc = zl3073x_read_id(zldev, &id);
> > > +		if (rc)
> > > +			return rc;
> > > +		rc = zl3073x_read_revision(zldev, &revision);
> > > +		if (rc)
> > > +			return rc;
> > > +		rc = zl3073x_read_fw_ver(zldev, &fw_ver);
> > > +		if (rc)
> > > +			return rc;
> > > +		rc = zl3073x_read_custom_config_ver(zldev, &cfg_ver);
> > > +		if (rc)
> > > +			return rc;
> > > +	}
> > 
> > Nothing improved here. Andrew comments are still valid and do not send
> > v3 before the discussion is resolved.
> 
> I'm accessing device registers here and they are protected by the device
> lock. I have to take the lock, register access functions expect this by
> lockdep_assert.

Because lockdep asserts is not a useful answer. Locks are there to
protect you from something. What are you protecting yourself from? If
you cannot answer that question, your locking scheme is built on sand
and very probably broken.

    Andrew