* [PATCHv2 net 0/2] fix the issue that may copy duplicate addrs into assoc's bind address list @ 2016-12-20 5:49 Xin Long 2016-12-20 5:49 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long 2016-12-20 5:49 ` [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long 0 siblings, 2 replies; 6+ messages in thread From: Xin Long @ 2016-12-20 5:49 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman Patch 1/2 is to fix some indent level. Given that we have kernels out there with this issue, patch 2/2 also fix sctp_raw_to_bind_addrs. v1 -> v2: Explain why we didn't filter the duplicate addresses when global address list gets updated in patch 2/2 changelog. Xin Long (2): sctp: reduce indent level in sctp_copy_local_addr_list sctp: not copying duplicate addrs to the assoc's bind address list net/sctp/bind_addr.c | 3 +++ net/sctp/protocol.c | 40 ++++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 18 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list 2016-12-20 5:49 [PATCHv2 net 0/2] fix the issue that may copy duplicate addrs into assoc's bind address list Xin Long @ 2016-12-20 5:49 ` Xin Long 2016-12-20 11:29 ` Marcelo Ricardo Leitner 2016-12-21 14:16 ` Neil Horman 2016-12-20 5:49 ` [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long 1 sibling, 2 replies; 6+ messages in thread From: Xin Long @ 2016-12-20 5:49 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman This patch is to reduce indent level by using continue when the addr is not allowed, and also drop end_copy by using break. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/sctp/protocol.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index 7b523e3..da5d82b 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -205,26 +205,27 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp, list_for_each_entry_rcu(addr, &net->sctp.local_addr_list, list) { if (!addr->valid) continue; - if (sctp_in_scope(net, &addr->a, scope)) { - /* Now that the address is in scope, check to see if - * the address type is really supported by the local - * sock as well as the remote peer. - */ - if ((((AF_INET == addr->a.sa.sa_family) && - (copy_flags & SCTP_ADDR4_PEERSUPP))) || - (((AF_INET6 == addr->a.sa.sa_family) && - (copy_flags & SCTP_ADDR6_ALLOWED) && - (copy_flags & SCTP_ADDR6_PEERSUPP)))) { - error = sctp_add_bind_addr(bp, &addr->a, - sizeof(addr->a), - SCTP_ADDR_SRC, GFP_ATOMIC); - if (error) - goto end_copy; - } - } + if (!sctp_in_scope(net, &addr->a, scope)) + continue; + + /* Now that the address is in scope, check to see if + * the address type is really supported by the local + * sock as well as the remote peer. + */ + if (addr->a.sa.sa_family == AF_INET && + !(copy_flags & SCTP_ADDR4_PEERSUPP)) + continue; + if (addr->a.sa.sa_family == AF_INET6 && + (!(copy_flags & SCTP_ADDR6_ALLOWED) || + !(copy_flags & SCTP_ADDR6_PEERSUPP))) + continue; + + error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a), + SCTP_ADDR_SRC, GFP_ATOMIC); + if (error) + break; } -end_copy: rcu_read_unlock(); return error; } -- 2.1.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list 2016-12-20 5:49 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long @ 2016-12-20 11:29 ` Marcelo Ricardo Leitner 2016-12-21 14:16 ` Neil Horman 1 sibling, 0 replies; 6+ messages in thread From: Marcelo Ricardo Leitner @ 2016-12-20 11:29 UTC (permalink / raw) To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman On Tue, Dec 20, 2016 at 01:49:49PM +0800, Xin Long wrote: > This patch is to reduce indent level by using continue when the addr > is not allowed, and also drop end_copy by using break. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > --- > net/sctp/protocol.c | 37 +++++++++++++++++++------------------ > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c > index 7b523e3..da5d82b 100644 > --- a/net/sctp/protocol.c > +++ b/net/sctp/protocol.c > @@ -205,26 +205,27 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp, > list_for_each_entry_rcu(addr, &net->sctp.local_addr_list, list) { > if (!addr->valid) > continue; > - if (sctp_in_scope(net, &addr->a, scope)) { > - /* Now that the address is in scope, check to see if > - * the address type is really supported by the local > - * sock as well as the remote peer. > - */ > - if ((((AF_INET == addr->a.sa.sa_family) && > - (copy_flags & SCTP_ADDR4_PEERSUPP))) || > - (((AF_INET6 == addr->a.sa.sa_family) && > - (copy_flags & SCTP_ADDR6_ALLOWED) && > - (copy_flags & SCTP_ADDR6_PEERSUPP)))) { > - error = sctp_add_bind_addr(bp, &addr->a, > - sizeof(addr->a), > - SCTP_ADDR_SRC, GFP_ATOMIC); > - if (error) > - goto end_copy; > - } > - } > + if (!sctp_in_scope(net, &addr->a, scope)) > + continue; > + > + /* Now that the address is in scope, check to see if > + * the address type is really supported by the local > + * sock as well as the remote peer. > + */ > + if (addr->a.sa.sa_family == AF_INET && > + !(copy_flags & SCTP_ADDR4_PEERSUPP)) > + continue; > + if (addr->a.sa.sa_family == AF_INET6 && > + (!(copy_flags & SCTP_ADDR6_ALLOWED) || > + !(copy_flags & SCTP_ADDR6_PEERSUPP))) > + continue; > + > + error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a), > + SCTP_ADDR_SRC, GFP_ATOMIC); > + if (error) > + break; > } > > -end_copy: > rcu_read_unlock(); > return error; > } > -- > 2.1.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list 2016-12-20 5:49 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long 2016-12-20 11:29 ` Marcelo Ricardo Leitner @ 2016-12-21 14:16 ` Neil Horman 1 sibling, 0 replies; 6+ messages in thread From: Neil Horman @ 2016-12-21 14:16 UTC (permalink / raw) To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner On Tue, Dec 20, 2016 at 01:49:49PM +0800, Xin Long wrote: > This patch is to reduce indent level by using continue when the addr > is not allowed, and also drop end_copy by using break. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/sctp/protocol.c | 37 +++++++++++++++++++------------------ > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c > index 7b523e3..da5d82b 100644 > --- a/net/sctp/protocol.c > +++ b/net/sctp/protocol.c > @@ -205,26 +205,27 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp, > list_for_each_entry_rcu(addr, &net->sctp.local_addr_list, list) { > if (!addr->valid) > continue; > - if (sctp_in_scope(net, &addr->a, scope)) { > - /* Now that the address is in scope, check to see if > - * the address type is really supported by the local > - * sock as well as the remote peer. > - */ > - if ((((AF_INET == addr->a.sa.sa_family) && > - (copy_flags & SCTP_ADDR4_PEERSUPP))) || > - (((AF_INET6 == addr->a.sa.sa_family) && > - (copy_flags & SCTP_ADDR6_ALLOWED) && > - (copy_flags & SCTP_ADDR6_PEERSUPP)))) { > - error = sctp_add_bind_addr(bp, &addr->a, > - sizeof(addr->a), > - SCTP_ADDR_SRC, GFP_ATOMIC); > - if (error) > - goto end_copy; > - } > - } > + if (!sctp_in_scope(net, &addr->a, scope)) > + continue; > + > + /* Now that the address is in scope, check to see if > + * the address type is really supported by the local > + * sock as well as the remote peer. > + */ > + if (addr->a.sa.sa_family == AF_INET && > + !(copy_flags & SCTP_ADDR4_PEERSUPP)) > + continue; > + if (addr->a.sa.sa_family == AF_INET6 && > + (!(copy_flags & SCTP_ADDR6_ALLOWED) || > + !(copy_flags & SCTP_ADDR6_PEERSUPP))) > + continue; > + > + error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a), > + SCTP_ADDR_SRC, GFP_ATOMIC); > + if (error) > + break; > } > > -end_copy: > rcu_read_unlock(); > return error; > } > -- > 2.1.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Series Acked-by: Neil Horman <nhorman@tuxdriver.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list 2016-12-20 5:49 [PATCHv2 net 0/2] fix the issue that may copy duplicate addrs into assoc's bind address list Xin Long 2016-12-20 5:49 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long @ 2016-12-20 5:49 ` Xin Long 2016-12-20 11:30 ` Marcelo Ricardo Leitner 1 sibling, 1 reply; 6+ messages in thread From: Xin Long @ 2016-12-20 5:49 UTC (permalink / raw) To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman sctp.local_addr_list is a global address list that is supposed to include all the local addresses. sctp updates this list according to NETDEV_UP/ NETDEV_DOWN notifications. However, if multiple NICs have the same address, the global list would have duplicate addresses. Even if for one NIC, promote secondaries in __inet_del_ifa can also lead to accumulating duplicate addresses. When sctp binds address 'ANY' and creates a connection, it copies all the addresses from global list into asoc's bind addr list, which makes sctp pack the duplicate addresses into INIT/INIT_ACK packets. This patch is to filter the duplicate addresses when copying the addrs from global list in sctp_copy_local_addr_list and unpacking addr_param from cookie in sctp_raw_to_bind_addrs to asoc's bind addr list. Note that we can't filter the duplicate addrs when global address list gets updated, As NETDEV_DOWN event may remove an addr that still exists in another NIC. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/sctp/bind_addr.c | 3 +++ net/sctp/protocol.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c index 401c607..1ebc184 100644 --- a/net/sctp/bind_addr.c +++ b/net/sctp/bind_addr.c @@ -292,6 +292,8 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list, } af->from_addr_param(&addr, rawaddr, htons(port), 0); + if (sctp_bind_addr_state(bp, &addr) != -1) + goto next; retval = sctp_add_bind_addr(bp, &addr, sizeof(addr), SCTP_ADDR_SRC, gfp); if (retval) { @@ -300,6 +302,7 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list, break; } +next: len = ntohs(param->length); addrs_len -= len; raw_addr_list += len; diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index da5d82b..616a942 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -220,6 +220,9 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp, !(copy_flags & SCTP_ADDR6_PEERSUPP))) continue; + if (sctp_bind_addr_state(bp, &addr->a) != -1) + continue; + error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a), SCTP_ADDR_SRC, GFP_ATOMIC); if (error) -- 2.1.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list 2016-12-20 5:49 ` [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long @ 2016-12-20 11:30 ` Marcelo Ricardo Leitner 0 siblings, 0 replies; 6+ messages in thread From: Marcelo Ricardo Leitner @ 2016-12-20 11:30 UTC (permalink / raw) To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman On Tue, Dec 20, 2016 at 01:49:50PM +0800, Xin Long wrote: > sctp.local_addr_list is a global address list that is supposed to include > all the local addresses. sctp updates this list according to NETDEV_UP/ > NETDEV_DOWN notifications. > > However, if multiple NICs have the same address, the global list would > have duplicate addresses. Even if for one NIC, promote secondaries in > __inet_del_ifa can also lead to accumulating duplicate addresses. > > When sctp binds address 'ANY' and creates a connection, it copies all > the addresses from global list into asoc's bind addr list, which makes > sctp pack the duplicate addresses into INIT/INIT_ACK packets. > > This patch is to filter the duplicate addresses when copying the addrs > from global list in sctp_copy_local_addr_list and unpacking addr_param > from cookie in sctp_raw_to_bind_addrs to asoc's bind addr list. > > Note that we can't filter the duplicate addrs when global address list > gets updated, As NETDEV_DOWN event may remove an addr that still exists > in another NIC. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > --- > net/sctp/bind_addr.c | 3 +++ > net/sctp/protocol.c | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c > index 401c607..1ebc184 100644 > --- a/net/sctp/bind_addr.c > +++ b/net/sctp/bind_addr.c > @@ -292,6 +292,8 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list, > } > > af->from_addr_param(&addr, rawaddr, htons(port), 0); > + if (sctp_bind_addr_state(bp, &addr) != -1) > + goto next; > retval = sctp_add_bind_addr(bp, &addr, sizeof(addr), > SCTP_ADDR_SRC, gfp); > if (retval) { > @@ -300,6 +302,7 @@ int sctp_raw_to_bind_addrs(struct sctp_bind_addr *bp, __u8 *raw_addr_list, > break; > } > > +next: > len = ntohs(param->length); > addrs_len -= len; > raw_addr_list += len; > diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c > index da5d82b..616a942 100644 > --- a/net/sctp/protocol.c > +++ b/net/sctp/protocol.c > @@ -220,6 +220,9 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp, > !(copy_flags & SCTP_ADDR6_PEERSUPP))) > continue; > > + if (sctp_bind_addr_state(bp, &addr->a) != -1) > + continue; > + > error = sctp_add_bind_addr(bp, &addr->a, sizeof(addr->a), > SCTP_ADDR_SRC, GFP_ATOMIC); > if (error) > -- > 2.1.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-21 14:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-12-20 5:49 [PATCHv2 net 0/2] fix the issue that may copy duplicate addrs into assoc's bind address list Xin Long 2016-12-20 5:49 ` [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long 2016-12-20 11:29 ` Marcelo Ricardo Leitner 2016-12-21 14:16 ` Neil Horman 2016-12-20 5:49 ` [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long 2016-12-20 11:30 ` Marcelo Ricardo Leitner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox