Re: [PATCH] net: ethernet: Fixe issue in nvmem_get_mac_address() where invalid mac addresses

On Thu, May 08, 2025 at 02:14:00AM +0000, Ozgur Kara wrote:

From: Ozgur Karatas <redacted>

it's necessary to log error returned from
fwnode_property_read_u8_array because there is no detailed information
when addr returns an invalid mac address.

Maybe "useful" would be better wording than "necessary".
Logging doesn't seem to be a hard requirement to me.

Moreover, I'm not sure that logging is appropriate.
E.g. fwnode_get_mac_addr() is called by fwnode_get_mac_address(),
which is in turn called by acpi_get_mac_address(), which logs if call
fails.

kfree(mac) should actually be marked as kfree((void *)mac) because mac
pointer is of type const void * and type conversion is required so
data returned from nvmem_cell_read() is of same type.

It seems to me that nvmem_cell_read returns void *.
So a good approach would be to change type of mac to void *.
In any case I don't think casting away const is the right approach;
what is the point of const if it is selectively ignored?

Also, I think this should be a separate patch to the logging change:
one patch per issue. If there is more than one patch then I would
suggest collecting them together in a patch-set with a cover letter.

This patch fixes the issue in nvmem_get_mac_address() where invalid
mac addresses could be read due to improper error handling.

I don't see any changes to the program flow for error handling in this patch.
It doesn't seem like a fix to me.

Signed-off-by: Ozgur Karatas <redacted>

-- 
pw-bot: changes-requested