| Message ID | 130956b1e880eab780162a795fde156d61d4de0f.1471605833.git.lucien.xin@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Aug 19, 2016 at 07:30:22PM +0800, Xin Long wrote: > From: Xin Long <lxin@redhat.com> > > 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 will > 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. > > Signed-off-by: Xin Long <lxin@redhat.com> Under what valid use case will multiple interfaces have the same network address? Neil > --- > 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 >
> Under what valid use case will multiple interfaces have the same network > address? > Hi, Neil. I'm not sure the specific valid use case. The point is, do we trust the sctp global addr list has no duplicate address ? In one of users' computer, he found hundreds of duplicate addresses in INIT_ACK packets. (https://bugzilla.redhat.com/show_bug.cgi?id=1308362) If we think NETDEV_UP/DOWN event in addrs chain always come in pairs, I can not explain how come this issue happened. But from sctp end, we can avoid it with this patch. after all binding any duplicate addresses is illegal, just like what sctp_bind does. what do you think ?
On Fri, Aug 19, 2016 at 07:30:22PM +0800, Xin Long wrote: > From: Xin Long <lxin@redhat.com> > > 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 will > 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. > > Signed-off-by: Xin Long <lxin@redhat.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 > Ah, I see what you're doing. Ok, this makes some sense, at least on the receive side, when you get a cookie unpacked and modify the remote peers address list, it makes sense to check for duplicates. On the local side however, I would, instead of checking it when the list gets copied, I'd check it when the master list gets updated (in the NETDEV_UP event notifier for the local address list, and the sctp_add_bind_addr function for the endpoint address list). That way you can keep that nested for loop out of the send path on the local system. Neil
> Ah, I see what you're doing. Ok, this makes some sense, at least on the receive > side, when you get a cookie unpacked and modify the remote peers address list, > it makes sense to check for duplicates. On the local side however, I would, > instead of checking it when the list gets copied, I'd check it when the master > list gets updated (in the NETDEV_UP event notifier for the local address list, I was thinking about to check it in the NETDEV_UP, yes it can make the master list has no duplicated addresses. But what if two same addresses events come, and they come from different NICs (though I can't point out the valid use case), then we filter there. Later, sctp may receive one NETDEV_DOWN event,sctp will remove that addr in the master list, but it shouldn't have been removed, as another local NIC still has that addr. That's why I have to leave the master alone, just check when they are really being bind to asoc addr list. > and the sctp_add_bind_addr function for the endpoint address list). That way As to the endpoint address list, sctp has different process for binding the address 'ANY' from assoc address list (note that this issue only happened in binding the address 'ANY'). instead of copying the master address list to the endpoint, it only adds address 'ANY' to the EP address list. Only when starting a connection and create the assoc, it copy the master address list to ASOC. So no need to do it in sctp_add_bind_addr for endpoint address list. Besides, sctp_add_bind_addr is supposed to be called after checking the duplicated address(I got it from sctp_do_bind()). :-) > you can keep that nested for loop out of the send path on the local system. > >
On Sat, Aug 20, 2016 at 02:41:01PM +0800, Xin Long wrote: > > Ah, I see what you're doing. Ok, this makes some sense, at least on the receive > > side, when you get a cookie unpacked and modify the remote peers address list, > > it makes sense to check for duplicates. On the local side however, I would, > > instead of checking it when the list gets copied, I'd check it when the master > > list gets updated (in the NETDEV_UP event notifier for the local address list, > > I was thinking about to check it in the NETDEV_UP, yes it can make the > master list has no duplicated addresses. But what if two same addresses > events come, and they come from different NICs (though I can't point out > the valid use case), then we filter there. > That I think would be a bug in the protocol code. For the ipv4 case, all addresses are owned by the system and the same addresses added to multiple interfaces should not be allowed. The same is true of ipv6 case. The only exception there is a link local address and that should still be unique within the context of an address/dev tuple. > Later, sctp may receive one NETDEV_DOWN event,sctp will remove that > addr in the master list, but it shouldn't have been removed, as another local > NIC still has that addr. > > That's why I have to leave the master alone, just check when they are really > being bind to asoc addr list. > > > and the sctp_add_bind_addr function for the endpoint address list). That way > > As to the endpoint address list, sctp has different process for binding > the address 'ANY' from assoc address list (note that this issue only > happened in binding the address 'ANY'). instead of copying the master > address list to the endpoint, it only adds address 'ANY' to the EP > address list. Only when starting a connection and create the assoc, it > copy the master address list to ASOC. > > So no need to do it in sctp_add_bind_addr for endpoint address list. > Besides, sctp_add_bind_addr is supposed to be called after checking > the duplicated address(I got it from sctp_do_bind()). :-) > > > you can keep that nested for loop out of the send path on the local system. > > > > >
>> > Ah, I see what you're doing. Ok, this makes some sense, at least on the receive >> > side, when you get a cookie unpacked and modify the remote peers address list, >> > it makes sense to check for duplicates. On the local side however, I would, >> > instead of checking it when the list gets copied, I'd check it when the master >> > list gets updated (in the NETDEV_UP event notifier for the local address list, >> >> I was thinking about to check it in the NETDEV_UP, yes it can make the >> master list has no duplicated addresses. But what if two same addresses >> events come, and they come from different NICs (though I can't point out >> the valid use case), then we filter there. >> > That I think would be a bug in the protocol code. For the ipv4 case, all > addresses are owned by the system and the same addresses added to multiple > interfaces should not be allowed. The same is true of ipv6 case. The only > exception there is a link local address and that should still be unique within > the context of an address/dev tuple. > understand, just sounds a little harsh. :-) For now, does it make sense to you to just leave as the master list is, and check the duplicate address when sctp is really binding them ?
On Wed, Aug 24, 2016 at 01:14:27PM +0800, Xin Long wrote: > >> > Ah, I see what you're doing. Ok, this makes some sense, at least on the receive > >> > side, when you get a cookie unpacked and modify the remote peers address list, > >> > it makes sense to check for duplicates. On the local side however, I would, > >> > instead of checking it when the list gets copied, I'd check it when the master > >> > list gets updated (in the NETDEV_UP event notifier for the local address list, > >> > >> I was thinking about to check it in the NETDEV_UP, yes it can make the > >> master list has no duplicated addresses. But what if two same addresses > >> events come, and they come from different NICs (though I can't point out > >> the valid use case), then we filter there. > >> > > That I think would be a bug in the protocol code. For the ipv4 case, all > > addresses are owned by the system and the same addresses added to multiple > > interfaces should not be allowed. The same is true of ipv6 case. The only > > exception there is a link local address and that should still be unique within > > the context of an address/dev tuple. > > > understand, just sounds a little harsh. :-) > > For now, does it make sense to you to just leave as the master list > is, and check > the duplicate address when sctp is really binding them ? > I would think so, yes. Neil
On Mon, Aug 22, 2016 at 10:25:38AM -0400, Neil Horman wrote: > On Sat, Aug 20, 2016 at 02:41:01PM +0800, Xin Long wrote: > > > Ah, I see what you're doing. Ok, this makes some sense, at least on the receive > > > side, when you get a cookie unpacked and modify the remote peers address list, > > > it makes sense to check for duplicates. On the local side however, I would, > > > instead of checking it when the list gets copied, I'd check it when the master > > > list gets updated (in the NETDEV_UP event notifier for the local address list, > > > > I was thinking about to check it in the NETDEV_UP, yes it can make the > > master list has no duplicated addresses. But what if two same addresses > > events come, and they come from different NICs (though I can't point out > > the valid use case), then we filter there. > > I guess a valid use case would be the poor man's roaming between wifi and ethernet with both mac addresses assigned to the same IP address, so that you don't terminate your connections when moving from one to another. This works quite well. It could even be just a temporary config during setup. Like, a sysadmin forgot to remove the address from a NIC before adding on the other one, and then noticed it. For a while, the system would have the address assigned to two interfaces. > That I think would be a bug in the protocol code. For the ipv4 case, all > addresses are owned by the system and the same addresses added to multiple > interfaces should not be allowed. The same is true of ipv6 case. The only > exception there is a link local address and that should still be unique within > the context of an address/dev tuple. > Maybe it should not but there is nothing stopping you from doing so. > > Later, sctp may receive one NETDEV_DOWN event,sctp will remove that > > addr in the master list, but it shouldn't have been removed, as another local > > NIC still has that addr. > > > > That's why I have to leave the master alone, just check when they are really > > being bind to asoc addr list. > > Or add a refcnt to its members. </idea> NETDEV_UP, it gets a ++ if it's already there NETDEV_DOWN, it gets a -- and cleans it up if it reaches 0 And the rest probably could stay the same. > > > and the sctp_add_bind_addr function for the endpoint address list). That way > > > > As to the endpoint address list, sctp has different process for binding > > the address 'ANY' from assoc address list (note that this issue only > > happened in binding the address 'ANY'). instead of copying the master > > address list to the endpoint, it only adds address 'ANY' to the EP > > address list. Only when starting a connection and create the assoc, it > > copy the master address list to ASOC. > > > > So no need to do it in sctp_add_bind_addr for endpoint address list. > > Besides, sctp_add_bind_addr is supposed to be called after checking > > the duplicated address(I got it from sctp_do_bind()). :-) > > > > > you can keep that nested for loop out of the send path on the local system. > > > > > > > > >
> Or add a refcnt to its members. </idea> > NETDEV_UP, it gets a ++ if it's already there > NETDEV_DOWN, it gets a -- and cleans it up if it reaches 0 > And the rest probably could stay the same. > Yes, it could also avoid the issue of amounts of duplicate addrs. or add a nic index variable to its members. But I still prefer the current patch. 1. This issue only happens when server bind 'ANY' addresses. we don't need to add any new members to struct sctp_sockaddr_entry. especially if it's a really corner issue, we fix this as an improvement. 2. It's yet two issues here, the duplicate addrs may be from a) different local NICs. b) the same one NIC. It may be unexpectable to filter them in NETDEV_UP/DOWN events. 3. We check it only when sctp really binds it, just like sctp_do_bind. What do you think ?
On Thu, Aug 25, 2016 at 12:03:30PM +0800, Xin Long wrote: > > Or add a refcnt to its members. </idea> > > NETDEV_UP, it gets a ++ if it's already there > > NETDEV_DOWN, it gets a -- and cleans it up if it reaches 0 > > And the rest probably could stay the same. > > > Yes, it could also avoid the issue of amounts of duplicate addrs. > or add a nic index variable to its members. > > But I still prefer the current patch. > 1. This issue only happens when server bind 'ANY' addresses. > we don't need to add any new members to struct sctp_sockaddr_entry. > especially if it's a really corner issue, we fix this as an improvement. > > 2. It's yet two issues here, the duplicate addrs may be from > a) different local NICs. > b) the same one NIC. > It may be unexpectable to filter them in NETDEV_UP/DOWN events. > > 3. We check it only when sctp really binds it, just like sctp_do_bind. > > What do you think ? Yep, +1. LGTM the current patch, thanks.
From: Of Xin Long > Sent: 25 August 2016 05:04 ... > But I still prefer the current patch. > 1. This issue only happens when server bind 'ANY' addresses. > we don't need to add any new members to struct sctp_sockaddr_entry. > especially if it's a really corner issue, we fix this as an improvement. > > 2. It's yet two issues here, the duplicate addrs may be from > a) different local NICs. > b) the same one NIC. > It may be unexpectable to filter them in NETDEV_UP/DOWN events. > > 3. We check it only when sctp really binds it, just like sctp_do_bind. > > What do you think ? I want to know what kind of weed the SCTP authors were smoking when they decided it was appropriate to put all of a systems IP addresses in the cookie and cookie-ack messages. It would be nice to have the local addresses selected by the kernel on the basis of the remote address - removing the necessity of the application to know the current network topology (and IP addresses) in order to bind to the correct local addresses before making an outward call. This sort of requires that local addresses for a connection are more of a property of the routing table than anything else. Consider the following network: ----+---------------+----------------------+--------- | | | x.x.1.1 x.x.1.2 y.y.1.2 10.1.1.1 10.1.1.2 10.1.1.2 | | | ----+---------------+ +--------- A connection from x.x.1.1 to x.x.1.2 needs to specify the 10.1.1.* addresses, but a connection for x.x.1.1 to y.y.1.2 must not. I'm not at sure whether it is possible to setup listener(s) on x.x.1.1 that can accept connections from both x.x.1.2 and y.y.1.2. David
On Fri, Sep 02, 2016 at 01:22:05PM +0000, David Laight wrote: > From: Of Xin Long > > Sent: 25 August 2016 05:04 > ... > > But I still prefer the current patch. > > 1. This issue only happens when server bind 'ANY' addresses. > > we don't need to add any new members to struct sctp_sockaddr_entry. > > especially if it's a really corner issue, we fix this as an improvement. > > > > 2. It's yet two issues here, the duplicate addrs may be from > > a) different local NICs. > > b) the same one NIC. > > It may be unexpectable to filter them in NETDEV_UP/DOWN events. > > > > 3. We check it only when sctp really binds it, just like sctp_do_bind. > > > > What do you think ? > > I want to know what kind of weed the SCTP authors were smoking when they > decided it was appropriate to put all of a systems IP addresses in the > cookie and cookie-ack messages. > The 'best effort' one I guess :-) > It would be nice to have the local addresses selected by the kernel on the > basis of the remote address - removing the necessity of the application > to know the current network topology (and IP addresses) in order to bind > to the correct local addresses before making an outward call. > > This sort of requires that local addresses for a connection are more of a > property of the routing table than anything else. > > Consider the following network: > > ----+---------------+----------------------+--------- > | | | > x.x.1.1 x.x.1.2 y.y.1.2 > 10.1.1.1 10.1.1.2 10.1.1.2 > | | | > ----+---------------+ +--------- > > A connection from x.x.1.1 to x.x.1.2 needs to specify the 10.1.1.* addresses, > but a connection for x.x.1.1 to y.y.1.2 must not. > Exactly. Your example is an exact match with the issue I had last week. That 10.1.1.2 of yours, may very well be 192.168.122.1 from libvirt. Then the host may attempt to send the packet to itself, and abort the association due to that. I hated this because it took me a big while to find out about it. I guess they just didn't predict this situation of duplicated/internal addresses. Otherwise, the address is there, reachable or not. It's in the best effort situation. > I'm not at sure whether it is possible to setup listener(s) on x.x.1.1 > that can accept connections from both x.x.1.2 and y.y.1.2. You mean without an explicit bind()? Marcelo
From: Marcelo Ricardo Leitner > Sent: 02 September 2016 14:47 ... > > Consider the following network: > > > > ----+---------------+----------------------+--------- > > | | | > > x.x.1.1 x.x.1.2 y.y.1.2 > > 10.1.1.1 10.1.1.2 10.1.1.2 > > | | | > > ----+---------------+ +--------- ... > > I'm not at sure whether it is possible to setup listener(s) on x.x.1.1 > > that can accept connections from both x.x.1.2 and y.y.1.2. > > You mean without an explicit bind()? You might be able to bind one socket to x.x.1.1 and a second to 10.1.1.1 and x.x.1.1 so that connections to x.x.1.1 are only offered x.x.1.1 and those to 10.1.1.1 are offered both addresses. But you can't make the init-ack responses for connections to x.x.1.1 depend on whether the connect came from x.x.1.2 or y.y.1.2. If the source and destination ports are fixed (as they usually are for M3UA) you can sit trying to make an outward connection, relying on the init collision code to work properly. Far from ideal! David
On Fri, Sep 02, 2016 at 02:25:42PM +0000, David Laight wrote: > From: Marcelo Ricardo Leitner > > Sent: 02 September 2016 14:47 > ... > > > Consider the following network: > > > > > > ----+---------------+----------------------+--------- > > > | | | > > > x.x.1.1 x.x.1.2 y.y.1.2 > > > 10.1.1.1 10.1.1.2 10.1.1.2 > > > | | | > > > ----+---------------+ +--------- > ... > > > I'm not at sure whether it is possible to setup listener(s) on x.x.1.1 > > > that can accept connections from both x.x.1.2 and y.y.1.2. > > > > You mean without an explicit bind()? > > You might be able to bind one socket to x.x.1.1 and a second to 10.1.1.1 > and x.x.1.1 so that connections to x.x.1.1 are only offered x.x.1.1 and > those to 10.1.1.1 are offered both addresses. > > But you can't make the init-ack responses for connections to x.x.1.1 > depend on whether the connect came from x.x.1.2 or y.y.1.2. > > If the source and destination ports are fixed (as they usually are for M3UA) > you can sit trying to make an outward connection, relying on the init collision > code to work properly. Far from ideal! Ahh yes, I see now. Yep.. Marcelo
On Wed, Aug 24, 2016 at 6:38 PM, Neil Horman <nhorman@tuxdriver.com> wrote: > On Wed, Aug 24, 2016 at 01:14:27PM +0800, Xin Long wrote: >> >> > Ah, I see what you're doing. Ok, this makes some sense, at least on the receive >> >> > side, when you get a cookie unpacked and modify the remote peers address list, >> >> > it makes sense to check for duplicates. On the local side however, I would, >> >> > instead of checking it when the list gets copied, I'd check it when the master >> >> > list gets updated (in the NETDEV_UP event notifier for the local address list, >> >> >> >> I was thinking about to check it in the NETDEV_UP, yes it can make the >> >> master list has no duplicated addresses. But what if two same addresses >> >> events come, and they come from different NICs (though I can't point out >> >> the valid use case), then we filter there. >> >> >> > That I think would be a bug in the protocol code. For the ipv4 case, all >> > addresses are owned by the system and the same addresses added to multiple >> > interfaces should not be allowed. The same is true of ipv6 case. The only >> > exception there is a link local address and that should still be unique within >> > the context of an address/dev tuple. >> > >> understand, just sounds a little harsh. :-) >> >> For now, does it make sense to you to just leave as the master list >> is, and check >> the duplicate address when sctp is really binding them ? >> > I would think so, yes. Hi, Neil, About this patch, I think we are on the page, right ? If yes, I will repost v2, but other than improving some changelog, no other change compare to v1. Do you agree ?
On Sat, Dec 17, 2016 at 05:56:51PM +0800, Xin Long wrote: > On Wed, Aug 24, 2016 at 6:38 PM, Neil Horman <nhorman@tuxdriver.com> wrote: > > On Wed, Aug 24, 2016 at 01:14:27PM +0800, Xin Long wrote: > >> >> > Ah, I see what you're doing. Ok, this makes some sense, at least on the receive > >> >> > side, when you get a cookie unpacked and modify the remote peers address list, > >> >> > it makes sense to check for duplicates. On the local side however, I would, > >> >> > instead of checking it when the list gets copied, I'd check it when the master > >> >> > list gets updated (in the NETDEV_UP event notifier for the local address list, > >> >> > >> >> I was thinking about to check it in the NETDEV_UP, yes it can make the > >> >> master list has no duplicated addresses. But what if two same addresses > >> >> events come, and they come from different NICs (though I can't point out > >> >> the valid use case), then we filter there. > >> >> > >> > That I think would be a bug in the protocol code. For the ipv4 case, all > >> > addresses are owned by the system and the same addresses added to multiple > >> > interfaces should not be allowed. The same is true of ipv6 case. The only > >> > exception there is a link local address and that should still be unique within > >> > the context of an address/dev tuple. > >> > > >> understand, just sounds a little harsh. :-) > >> > >> For now, does it make sense to you to just leave as the master list > >> is, and check > >> the duplicate address when sctp is really binding them ? > >> > > I would think so, yes. > > Hi, Neil, > > About this patch, I think we are on the page, right ? > Yes, I think we are. Neil > If yes, I will repost v2, but other than improving some changelog, > no other change compare to v1. Do you agree ? >
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)