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

[Simon Horman]

On Thu, May 08, 2025 at 02:14:00AM +0000, Ozgur Kara wrote:
> From: Ozgur Karatas <ozgur@xxxxxxxxxx>
> 
> 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 <ozgur@xxxxxxxxxx>

-- 
pw-bot: changes-requested