On 02/10/20 7:45 am, David Miller wrote: > From: Anant Thazhemadam <anant.thazhemadam@xxxxxxxxx> > Date: Thu, 1 Oct 2020 13:02:20 +0530 > >> When get_registers() fails (which happens when usb_control_msg() fails) >> in set_ethernet_addr(), the uninitialized value of node_id gets copied >> as the address. >> >> Checking for the return values appropriately, and handling the case >> wherein set_ethernet_addr() fails like this, helps in avoiding the >> mac address being incorrectly set in this manner. >> >> Reported-by: syzbot+abbc768b560c84d92fd3@xxxxxxxxxxxxxxxxxxxxxxxxx >> Tested-by: syzbot+abbc768b560c84d92fd3@xxxxxxxxxxxxxxxxxxxxxxxxx >> Signed-off-by: Anant Thazhemadam <anant.thazhemadam@xxxxxxxxx> >> Acked-by: Petko Manolov <petkan@xxxxxxxxxxxxx> > First, please remove "Linux-kernel-mentees" from the Subject line. > > All patch submitters should have their work judged equally, whoever > they are. So this Subject text gives no extra information, and it > simply makes scanning Subject lines in one's mailer more difficult. I will keep that in mind for all future submissions. Thank you. > Second, when a MAC address fails to probe a random MAC address should > be selected. We have helpers for this. This way an interface still > comes up and is usable, even in the event of a failed MAC address > probe. Okay... I see. But this patch is about ensuring that an uninitialized variable's value (whatever that may be) is not set as the ethernet address blindly (without any form of checking if get_registers() worked as expected, or not). And I didn't think uninitialized values being set as MAC address was considered a good outcome (after all, it seemed to have triggered a bug), especially when it could have been avoided by introducing a simple check that doesn't break anything. However, if I was mistaken, and if that is something that we can live with after all, then I don't really see the understand the purpose of similar checks being made (in all the many places that the return value of get_registers() (or a similar function gets checked) in the first place at all. In all honesty, this confused me a little more than it provided clarity, and I hope someone could help me shift that balance back to clarity again. Thank you for your time. Thanks, Anant
