On Sat, Sep 26, 2015 at 02:09:03PM -0400, Tejun Heo wrote:
quoted
@@ -1628,7 +1632,7 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr,
nladdr->nl_pid = nlk->dst_portid;
nladdr->nl_groups = netlink_group_mask(nlk->dst_group);
} else {
- nladdr->nl_pid = nlk->portid;
+ nladdr->nl_pid = netlink_bound(nlk) ? nlk->portid : 0;
nladdr->nl_groups = nlk->groups ? nlk->groups[0] : 0;
}
return 0;
So, this is really weird because netlink_getname() doens't participate
in the autobind race and thus it's perfectly fine for it to not worry
about whether ->bound is set or the memory barrier - whoever its
caller may be, the caller is of course responsible for ensuring that
the port is bound and visible if it expects to read back the number -
ie. if the caller doesn't know (in memory ordering sense) that
bind/connect/sendmsg succeeded, it of course can't expect to reliably
read back the port number. getname never needed the barrier. The
above is shifting synchronization from the source to its users. This
is a bad thing to do.
Thread 1 Thread 2
sendmsg getsockname
netlink_autobind netlink_getname
Thread 2 should not have to do anything special to guarantee that
getsockname does not return garbage. It must either be the bound
portid if the autobind completed in thread 1 and is visible or it
should return zero.
As it stands thread 2 may see a portid belonging to somebody else
if it catches the autobind in thread 1 trying different portids
while roving.
Cheers,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt