linux-kernel - Re: [PATCH v6 3/3] nvmem: core: Expose cells through sysfs

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: <20230731180531.7c616b3b@xps-13>
Date:   Mon, 31 Jul 2023 18:05:31 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Chen-Yu Tsai <wenst@...omium.org>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Robert Marko <robert.marko@...tura.hr>,
        Luka Perkov <luka.perkov@...tura.hr>,
        Michael Walle <michael@...le.cc>,
        Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH v6 3/3] nvmem: core: Expose cells through sysfs

Hi Chen-Yu,

> >  static int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> > @@ -1006,6 +1101,12 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> >  	if (rval)
> >  		goto err_remove_cells;
> >  
> > +#ifdef CONFIG_NVMEM_SYSFS
> > +	rval = nvmem_populate_sysfs_cells(nvmem);
> > +	if (rval)
> > +		goto err_remove_cells;  
> 
> This breaks nvmem / efuse devices with multiple cells that share the
> same name. Something like this in DT:
> 
> 	efuse: efuse@...10000 {
> 		compatible = "mediatek,mt8183-efuse",
> 			     "mediatek,efuse";
> 		reg = <0 0x11f10000 0 0x1000>;
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		thermal_calibration: calib@180 {
> 			reg = <0x180 0xc>;
> 		};
> 
> 		mipi_tx_calibration: calib@190 {
> 			reg = <0x190 0xc>;
> 		};
> 
> 		svs_calibration: calib@580 {
> 			reg = <0x580 0x64>;
> 		};
> 	};
> 
> creates three cells, all named DT, and sysfs will complain:
> 
>     sysfs: cannot create duplicate filename '/devices/platform/soc/11f10000.efuse/nvmem1/cells/calib'
>     mediatek,efuse: probe of 11f10000.efuse failed with error -17
> 
> This causes the MT8183-based Chromebooks to lose display capability,
> among other things.

Sorry for the breakage, I did not identify this case, but you're right
this is incorrectly handled currently.

> The problem lies in the nvmem DT parsing code, where the cell name is
> derived from the node name, without including the address portion.
> However I'm not sure we can change that, since it could be considered
> ABI?

I would be in favor suffixing the cell names anyway as they have not
been exposed yet to userspace at all (well, not more than a couple of
days in -next).

Thanks,
Miquèl