Thread (16 messages) 16 messages, 5 authors, 2020-10-18

Re: [Linux-kernel-mentees] [PATCH] net: usb: rtl8150: don't incorrectly assign random MAC addresses

From: Jakub Kicinski <kuba@kernel.org>
Date: 2020-10-10 23:12:53
Also in: linux-next, linux-usb, lkml, netdev

On Sun, 11 Oct 2020 00:14:05 +0530 Anant Thazhemadam wrote:
Ah, my apologies. You're right. It doesn't look like those helpers have made
their way into the networking tree yet.

(This gets mentioned here as well,
    https://www.mail-archive.com/netdev@vger.kernel.org/msg357843.html)

The commit ID pointed to by the fixes tag is correct.
The change introduced by said commit looks right, but is logically incorrect.

get_registers() directly returns the return value of usb_control_msg_recv(),
and usb_control_msg_recv() returns 0 on success and negative error number
otherwise.

(You can find more about the new helpers here
    https://lore.kernel.org/alsa-devel/20200914153756.3412156-1-gregkh@linuxfoundation.org/ (local) )

The commit ID mentioned introduces a change that is supposed to copy over
the ethernet only when get_registers() succeeds, i.e., a complete read occurs,
and generate and set a random ethernet address otherwise (reading the
commit message should give some more insight).

The condition that checks if get_registers() succeeds (as specified in f45a4248ea4c)
was,
    ret == sizeof(node_id)
where ret is the return value of get_registers().

However, ret will never equal sizeof(node_id), since ret can only be equal to 0
or a negative number.

Thus, even in case where get_registers() succeeds, a randomly generated MAC
address would get copied over, instead of copying the appropriate ethernet
address, which is logically incorrect and not optimal.

Hence, we need to modify this to check if (ret == 0), and copy over the correct
ethernet address in that case, instead of randomly generating one and assigning
that.
I see... so we ended up with your fix applied to net, and Petko's
rework applied to the usb/usb-next tree.

What you're actually fixing is the improper resolution of the resulting
conflict in linux-next!

CCing Stephen and linux-next.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help