linux-kernel - Re: [PATCH 2/4] thermal: imx: Add support for reading OCOTP through nvmem

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]

Date:   Fri, 14 Jul 2017 09:48:49 +0100
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Shawn Guo <shawnguo@...nel.org>,
        Leonard Crestez <leonard.crestez@....com>
Cc:     Zhang Rui <rui.zhang@...el.com>,
        Eduardo Valentin <edubezval@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Lothar Waßmann <LW@...O-electronics.de>,
        Fabio Estevam <fabio.estevam@....com>,
        Dong Aisheng <aisheng.dong@....com>,
        Bai Ping <ping.bai@....com>, Anson Huang <Anson.Huang@....com>,
        Octavian Purdila <octavian.purdila@....com>,
        linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] thermal: imx: Add support for reading OCOTP through
 nvmem



On 12/07/17 07:36, Shawn Guo wrote:
>>   
>> +static int nvmem_cell_read_u32(struct device* dev, const char *cell_id, u32 *val)
>> +{
>> +	struct nvmem_cell *cell;
>> +	void *buf;
>> +	size_t len;
>> +
>> +	cell = nvmem_cell_get(dev, cell_id);
>> +	if (IS_ERR(cell))
>> +		return PTR_ERR(cell);
>> +
>> +	buf = nvmem_cell_read(cell, &len);
>> +	if (IS_ERR(buf)) {
>> +		nvmem_cell_put(cell);
>> +		return PTR_ERR(buf);
>> +	}
>> +	if (len != sizeof(*val)) {
>> +		kfree(buf);
>> +		nvmem_cell_put(cell);
>> +		return -EINVAL;
>> +	}
>> +	memcpy(val, buf, sizeof(*val));

This can overflow the memory allocated to val, we should be careful here 
not to do so.
limit this to sizeof(u32) should be good. Also add some sanity checks to 
make sure that len is atleast 4 bytes.


>> +
>> +	kfree(buf);
>> +	nvmem_cell_put(cell);
>> +	return 0;
>> +}
> The function looks nothing IMX specific, and could be a nvmem core
> function?
> 
> @Srinivas, thoughts?
Yep, this function looks generic, can be moved to nvmem layer.

Thanks,
srini