From: Vlad Yasevich <vyasevich@gmail.com>
To: Daniel Borkmann <dborkman@redhat.com>, davem@davemloft.net
Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
Wang Weidong <wangweidong1@huawei.com>,
Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH net] net: sctp: fix copying sk_v6_rcv_saddr in sctp_v6_create_accept_sk
Date: Tue, 19 Nov 2013 15:38:10 +0000 [thread overview]
Message-ID: <528B85E2.5080403@gmail.com> (raw)
In-Reply-To: <1384858284-2246-1-git-send-email-dborkman@redhat.com>
On 11/19/2013 05:51 AM, Daniel Borkmann wrote:
> Wang reported an issue that lksctp's test_getname_v6 seems to fail.
>
> The issue is that we do not copy sk_v6_rcv_saddr over to the new
> socket, although the comment above says so regarding rcv_saddr.
>
> Commit 914e1c8b6980 ("sctp: Inherit all socket options from parent
> correctly.") originally moved that over to sctp_copy_sock(), but
> after commit efe4208f47f9 ("ipv6: make lookups simpler and faster")
> this no longer holds and the actual value of sk_v6_rcv_saddr was
> no longer being migrated.
>
> With this patch, the lksctp test suite passes again for IPv6.
>
> Fixes: efe4208f47f9 ("ipv6: make lookups simpler and faster")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/sctp/ipv6.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 7567e6f..be08592 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -661,6 +661,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
> * and getpeername().
> */
> sctp_v6_to_sk_daddr(&asoc->peer.primary_addr, newsk);
> + newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
>
> sk_refcnt_debug_inc(newsk);
>
>
This fixes the issue for the accept() case, but the bug is still there
in the peeloff case.
I think you should make sctp_copy_sock() use sock_copy() for now. It
looks like it will catch all the cases.
We can then look at possibly getting rid of sctp_copy_sock() for net-next.
-vlad
WARNING: multiple messages have this Message-ID (diff)
From: Vlad Yasevich <vyasevich@gmail.com>
To: Daniel Borkmann <dborkman@redhat.com>, davem@davemloft.net
Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
Wang Weidong <wangweidong1@huawei.com>,
Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH net] net: sctp: fix copying sk_v6_rcv_saddr in sctp_v6_create_accept_sk
Date: Tue, 19 Nov 2013 10:38:10 -0500 [thread overview]
Message-ID: <528B85E2.5080403@gmail.com> (raw)
In-Reply-To: <1384858284-2246-1-git-send-email-dborkman@redhat.com>
On 11/19/2013 05:51 AM, Daniel Borkmann wrote:
> Wang reported an issue that lksctp's test_getname_v6 seems to fail.
>
> The issue is that we do not copy sk_v6_rcv_saddr over to the new
> socket, although the comment above says so regarding rcv_saddr.
>
> Commit 914e1c8b6980 ("sctp: Inherit all socket options from parent
> correctly.") originally moved that over to sctp_copy_sock(), but
> after commit efe4208f47f9 ("ipv6: make lookups simpler and faster")
> this no longer holds and the actual value of sk_v6_rcv_saddr was
> no longer being migrated.
>
> With this patch, the lksctp test suite passes again for IPv6.
>
> Fixes: efe4208f47f9 ("ipv6: make lookups simpler and faster")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/sctp/ipv6.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 7567e6f..be08592 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -661,6 +661,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
> * and getpeername().
> */
> sctp_v6_to_sk_daddr(&asoc->peer.primary_addr, newsk);
> + newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
>
> sk_refcnt_debug_inc(newsk);
>
>
This fixes the issue for the accept() case, but the bug is still there
in the peeloff case.
I think you should make sctp_copy_sock() use sock_copy() for now. It
looks like it will catch all the cases.
We can then look at possibly getting rid of sctp_copy_sock() for net-next.
-vlad
next prev parent reply other threads:[~2013-11-19 15:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-19 10:51 [PATCH net] net: sctp: fix copying sk_v6_rcv_saddr in sctp_v6_create_accept_sk Daniel Borkmann
2013-11-19 10:51 ` Daniel Borkmann
2013-11-19 11:08 ` wangweidong
2013-11-19 11:08 ` wangweidong
2013-11-19 15:38 ` Vlad Yasevich [this message]
2013-11-19 15:38 ` Vlad Yasevich
2013-11-19 15:46 ` Daniel Borkmann
2013-11-19 15:46 ` Daniel Borkmann
2013-11-19 21:10 ` David Miller
2013-11-19 21:10 ` David Miller
2013-11-21 6:49 ` wangweidong
2013-11-21 6:49 ` wangweidong
2013-11-21 9:08 ` Daniel Borkmann
2013-11-21 9:08 ` Daniel Borkmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=528B85E2.5080403@gmail.com \
--to=vyasevich@gmail.com \
--cc=davem@davemloft.net \
--cc=dborkman@redhat.com \
--cc=eric.dumazet@gmail.com \
--cc=linux-sctp@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=wangweidong1@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.