| Message ID | 20190827164955.9249-1-bmt@zurich.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] RDMA/siw: Fix IPv6 addr_list locking | expand |
On Tue, Aug 27, 2019 at 06:49:55PM +0200, Bernard Metzler wrote: > Walking the address list of an inet6_dev requires > appropriate locking. Since the called function > siw_listen_address() may sleep, we have to use > rtnl_lock() instead of read_lock_bh(). > > Also introduces: > - sanity checks if we got a device from > in_dev_get() or in6_dev_get(). > - skipping IPv6 addresses flagged IFA_F_TENTATIVE > or IFA_F_DEPRECATED > > Reported-by: Bart Van Assche <bvanassche@acm.org> > Fixes: 6c52fdc244b5 ("rdma/siw: connection management") > Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> > drivers/infiniband/sw/siw/siw_cm.c | 33 +++++++++++++++++++----------- > 1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c > index 1db5ad3d9580..c145b4ff4556 100644 > +++ b/drivers/infiniband/sw/siw/siw_cm.c > @@ -1962,6 +1962,10 @@ int siw_create_listen(struct iw_cm_id *id, int backlog) > struct sockaddr_in s_laddr, *s_raddr; > const struct in_ifaddr *ifa; > > + if (!in_dev) { > + rv = -ENODEV; > + goto out; > + } > memcpy(&s_laddr, &id->local_addr, sizeof(s_laddr)); > s_raddr = (struct sockaddr_in *)&id->remote_addr; > > @@ -1991,22 +1995,27 @@ int siw_create_listen(struct iw_cm_id *id, int backlog) > struct sockaddr_in6 *s_laddr = &to_sockaddr_in6(id->local_addr), > *s_raddr = &to_sockaddr_in6(id->remote_addr); > > + if (!in6_dev) { > + rv = -ENODEV; > + goto out; > + } > siw_dbg(id->device, > "laddr %pI6:%d, raddr %pI6:%d\n", > &s_laddr->sin6_addr, ntohs(s_laddr->sin6_port), > &s_raddr->sin6_addr, ntohs(s_raddr->sin6_port)); > > - read_lock_bh(&in6_dev->lock); > - list_for_each_entry(ifp, &in6_dev->addr_list, if_list) { > - struct sockaddr_in6 bind_addr; > - > + rtnl_lock(); > + list_for_each_entry_rcu(ifp, &in6_dev->addr_list, if_list) { If not holding RCU then don't use the rcu list iterator.. Jason
-----"Jason Gunthorpe" <jgg@ziepe.ca> wrote: ----- >To: "Bernard Metzler" <bmt@zurich.ibm.com> >From: "Jason Gunthorpe" <jgg@ziepe.ca> >Date: 08/27/2019 07:28PM >Cc: linux-rdma@vger.kernel.org, bvanassche@acm.org, >dledford@redhat.com >Subject: [EXTERNAL] Re: [PATCH v2] RDMA/siw: Fix IPv6 addr_list >locking > >On Tue, Aug 27, 2019 at 06:49:55PM +0200, Bernard Metzler wrote: >> Walking the address list of an inet6_dev requires >> appropriate locking. Since the called function >> siw_listen_address() may sleep, we have to use >> rtnl_lock() instead of read_lock_bh(). >> >> Also introduces: >> - sanity checks if we got a device from >> in_dev_get() or in6_dev_get(). >> - skipping IPv6 addresses flagged IFA_F_TENTATIVE >> or IFA_F_DEPRECATED >> >> Reported-by: Bart Van Assche <bvanassche@acm.org> >> Fixes: 6c52fdc244b5 ("rdma/siw: connection management") >> Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> >> drivers/infiniband/sw/siw/siw_cm.c | 33 >+++++++++++++++++++----------- >> 1 file changed, 21 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/infiniband/sw/siw/siw_cm.c >b/drivers/infiniband/sw/siw/siw_cm.c >> index 1db5ad3d9580..c145b4ff4556 100644 >> +++ b/drivers/infiniband/sw/siw/siw_cm.c >> @@ -1962,6 +1962,10 @@ int siw_create_listen(struct iw_cm_id *id, >int backlog) >> struct sockaddr_in s_laddr, *s_raddr; >> const struct in_ifaddr *ifa; >> >> + if (!in_dev) { >> + rv = -ENODEV; >> + goto out; >> + } >> memcpy(&s_laddr, &id->local_addr, sizeof(s_laddr)); >> s_raddr = (struct sockaddr_in *)&id->remote_addr; >> >> @@ -1991,22 +1995,27 @@ int siw_create_listen(struct iw_cm_id *id, >int backlog) >> struct sockaddr_in6 *s_laddr = &to_sockaddr_in6(id->local_addr), >> *s_raddr = &to_sockaddr_in6(id->remote_addr); >> >> + if (!in6_dev) { >> + rv = -ENODEV; >> + goto out; >> + } >> siw_dbg(id->device, >> "laddr %pI6:%d, raddr %pI6:%d\n", >> &s_laddr->sin6_addr, ntohs(s_laddr->sin6_port), >> &s_raddr->sin6_addr, ntohs(s_raddr->sin6_port)); >> >> - read_lock_bh(&in6_dev->lock); >> - list_for_each_entry(ifp, &in6_dev->addr_list, if_list) { >> - struct sockaddr_in6 bind_addr; >> - >> + rtnl_lock(); >> + list_for_each_entry_rcu(ifp, &in6_dev->addr_list, if_list) { > >If not holding RCU then don't use the rcu list iterator.. > absolutely!
diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c index 1db5ad3d9580..c145b4ff4556 100644 --- a/drivers/infiniband/sw/siw/siw_cm.c +++ b/drivers/infiniband/sw/siw/siw_cm.c @@ -1962,6 +1962,10 @@ int siw_create_listen(struct iw_cm_id *id, int backlog) struct sockaddr_in s_laddr, *s_raddr; const struct in_ifaddr *ifa; + if (!in_dev) { + rv = -ENODEV; + goto out; + } memcpy(&s_laddr, &id->local_addr, sizeof(s_laddr)); s_raddr = (struct sockaddr_in *)&id->remote_addr; @@ -1991,22 +1995,27 @@ int siw_create_listen(struct iw_cm_id *id, int backlog) struct sockaddr_in6 *s_laddr = &to_sockaddr_in6(id->local_addr), *s_raddr = &to_sockaddr_in6(id->remote_addr); + if (!in6_dev) { + rv = -ENODEV; + goto out; + } siw_dbg(id->device, "laddr %pI6:%d, raddr %pI6:%d\n", &s_laddr->sin6_addr, ntohs(s_laddr->sin6_port), &s_raddr->sin6_addr, ntohs(s_raddr->sin6_port)); - read_lock_bh(&in6_dev->lock); - list_for_each_entry(ifp, &in6_dev->addr_list, if_list) { - struct sockaddr_in6 bind_addr; - + rtnl_lock(); + list_for_each_entry_rcu(ifp, &in6_dev->addr_list, if_list) { + if (ifp->flags & (IFA_F_TENTATIVE | IFA_F_DEPRECATED)) + continue; if (ipv6_addr_any(&s_laddr->sin6_addr) || ipv6_addr_equal(&s_laddr->sin6_addr, &ifp->addr)) { - bind_addr.sin6_family = AF_INET6; - bind_addr.sin6_port = s_laddr->sin6_port; - bind_addr.sin6_flowinfo = 0; - bind_addr.sin6_addr = ifp->addr; - bind_addr.sin6_scope_id = dev->ifindex; + struct sockaddr_in6 bind_addr = { + .sin6_family = AF_INET6, + .sin6_port = s_laddr->sin6_port, + .sin6_flowinfo = 0, + .sin6_addr = ifp->addr, + .sin6_scope_id = dev->ifindex }; rv = siw_listen_address(id, backlog, (struct sockaddr *)&bind_addr, @@ -2015,12 +2024,12 @@ int siw_create_listen(struct iw_cm_id *id, int backlog) listeners++; } } - read_unlock_bh(&in6_dev->lock); - + rtnl_unlock(); in6_dev_put(in6_dev); } else { - return -EAFNOSUPPORT; + rv = -EAFNOSUPPORT; } +out: if (listeners) rv = 0; else if (!rv)
Walking the address list of an inet6_dev requires appropriate locking. Since the called function siw_listen_address() may sleep, we have to use rtnl_lock() instead of read_lock_bh(). Also introduces: - sanity checks if we got a device from in_dev_get() or in6_dev_get(). - skipping IPv6 addresses flagged IFA_F_TENTATIVE or IFA_F_DEPRECATED Reported-by: Bart Van Assche <bvanassche@acm.org> Fixes: 6c52fdc244b5 ("rdma/siw: connection management") Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> --- drivers/infiniband/sw/siw/siw_cm.c | 33 +++++++++++++++++++----------- 1 file changed, 21 insertions(+), 12 deletions(-)