Thread (3 messages) flat view 3 messages, 2 authors, 2021-02-02

Re: [Patch net-next v2] net: fix dev_ifsioc_locked() race condition

From: Cong Wang <hidden>
Date: 2021-02-02 05:33:01

On Mon, Feb 1, 2021 at 7:40 PM Jakub Kicinski [off-list ref] wrote:
On Sat, 30 Jan 2021 18:27:55 -0800 Cong Wang wrote:
quoted
From: Cong Wang <redacted>

dev_ifsioc_locked() is called with only RCU read lock, so when
there is a parallel writer changing the mac address, it could
get a partially updated mac address, as shown below:

Thread 1                      Thread 2
// eth_commit_mac_addr_change()
memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
                              // dev_ifsioc_locked()
                              memcpy(ifr->ifr_hwaddr.sa_data,
                                      dev->dev_addr,...);

Close this race condition by guarding them with a RW semaphore,
like netdev_get_name(). The writers take RTNL anyway, so this
will not affect the slow path. To avoid bothering existing
dev_set_mac_address() callers in drivers, introduce a new wrapper
just for user-facing callers in ioctl and rtnetlink.
Some of the drivers need to be update, tho, right? At a quick look at
least bond and tun seem to be making calls to dev_set_mac_address()
on IOCTL paths.
Ah, good catch! Clearly I missed those special IOCTL's.

Thanks,
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help