fix the issue that may copy duplicate addrs into assoc's bind address list
Subsystems (3)
Xin Long
maintainerv2 cover fix the issue that may copy duplicate addrs into assoc's bind address list
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(-)
Signature · 1 line
2.1.0
Xin Long
maintainerPATCH v2 1/2 sctp: reduce indent level in sctp_copy_local_addr_list
This patch is to reduce indent level by using continue when the addr is not allowed, and also drop end_copy by using break.
--- net/sctp/protocol.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)
net/sctp/protocol.c +19 -18
@@ -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; }
Signature · 1 line
2.1.0
Xin Long
maintainerPATCH v2 2/2 sctp: not copying duplicate addrs to the assoc's bind address list
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.
--- net/sctp/bind_addr.c | 3 +++ net/sctp/protocol.c | 3 +++ 2 files changed, 6 insertions(+)
net/sctp/bind_addr.c +3 -0
@@ -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;
net/sctp/protocol.c +3 -0
@@ -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)
Signature · 1 line
2.1.0
Re: [PATCHv2 net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list
Quoted 16 lines · Xin Long
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_DOWNnotifications.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_ifacan 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_listand unpackingaddr_paramfrom cookie insctp_raw_to_bind_addrsto asoc's bind addr list.
Quoted 5 lines
Note that we can't filter the duplicate addrs when global address list gets updated, As
NETDEV_DOWNevent may remove an addr that still exists in another NIC.Signed-off-by Xin Long <[email protected]>
Quoted 48 lines
--- net/sctp/bind_addr.c | 3 +++ net/sctp/protocol.c | 3 +++ 2 files changed, 6 insertions(+)net/sctp/bind_addr.c +3
-0@@ -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;net/sctp/protocol.c +3
-0@@ -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)Signature · 6 lines
2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list
In reply to Xin LongQuoted 4 lines · Xin Long
This patch is to reduce indent level by using continue when the addr is not allowed, and also drop
end_copyby using break.Signed-off-by Xin Long <[email protected]>
Quoted 62 lines
--- net/sctp/protocol.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)net/sctp/protocol.c +19
-18@@ -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; }Signature · 6 lines
2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list
In reply to Xin LongQuoted 66 lines · Xin Long
This patch is to reduce indent level by using continue when the addr is not allowed, and also drop
end_copyby using break.Signed-off-by Xin Long <[email protected]>--- net/sctp/protocol.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)net/sctp/protocol.c +19
-18@@ -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; }Signature · 6 lines
2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Series