All of lore.kernel.org
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: Xin Long <lucien.xin@gmail.com>
Cc: network dev <netdev@vger.kernel.org>,
	linux-sctp@vger.kernel.org, davem@davemloft.net,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	syzkaller@googlegroups.com
Subject: Re: [PATCH net] sctp: not copy sctp_sock pd_lobby in sctp_copy_descendant
Date: Tue, 19 Mar 2019 07:13:00 -0400	[thread overview]
Message-ID: <20190319111300.GB3722@hmswarspite.think-freely.org> (raw)
In-Reply-To: <130ce0bbbc015f9fb47f97b51c650843e2ac39a3.1552910309.git.lucien.xin@gmail.com>

On Mon, Mar 18, 2019 at 07:58:29PM +0800, Xin Long wrote:
> Now sctp_copy_descendant() copies pd_lobby from old sctp scok to new
> sctp sock. If sctp_sock_migrate() returns error, it will panic when
> releasing new sock and trying to purge pd_lobby due to the incorrect
> pointers in pd_lobby.
> 
>   [  120.485116] kasan: CONFIG_KASAN_INLINE enabled
>   [  120.486270] kasan: GPF could be caused by NULL-ptr deref or user
>   [  120.509901] Call Trace:
>   [  120.510443]  sctp_ulpevent_free+0x1e8/0x490 [sctp]
>   [  120.511438]  sctp_queue_purge_ulpevents+0x97/0xe0 [sctp]
>   [  120.512535]  sctp_close+0x13a/0x700 [sctp]
>   [  120.517483]  inet_release+0xdc/0x1c0
>   [  120.518215]  __sock_release+0x1d2/0x2a0
>   [  120.519025]  sctp_do_peeloff+0x30f/0x3c0 [sctp]
> 
> We fix it by not copying sctp_sock pd_lobby in sctp_copy_descendan(),
> and skb_queue_head_init() can also be removed in sctp_sock_migrate().
> 
> Reported-by: syzbot+85e0b422ff140b03672a@syzkaller.appspotmail.com
> Fixes: 89664c623617 ("sctp: sctp_sock_migrate() returns error if sctp_bind_addr_dup() fails")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/socket.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 6140471..65b5386 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -9169,7 +9169,7 @@ static inline void sctp_copy_descendant(struct sock *sk_to,
>  {
>  	int ancestor_size = sizeof(struct inet_sock) +
>  			    sizeof(struct sctp_sock) -
> -			    offsetof(struct sctp_sock, auto_asconf_list);
> +			    offsetof(struct sctp_sock, pd_lobby);
>  
>  	if (sk_from->sk_family == PF_INET6)
>  		ancestor_size += sizeof(struct ipv6_pinfo);
> @@ -9253,7 +9253,6 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
>  	 * 2) Peeling off partial delivery; keep pd_lobby in new pd_lobby.
>  	 * 3) Peeling off non-partial delivery; move pd_lobby to receive_queue.
>  	 */
> -	skb_queue_head_init(&newsp->pd_lobby);
>  	atomic_set(&sctp_sk(newsk)->pd_mode, assoc->ulpq.pd_mode);
>  
>  	if (atomic_read(&sctp_sk(oldsk)->pd_mode)) {
> -- 
> 2.1.0
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

WARNING: multiple messages have this Message-ID (diff)
From: Neil Horman <nhorman@tuxdriver.com>
To: Xin Long <lucien.xin@gmail.com>
Cc: network dev <netdev@vger.kernel.org>,
	linux-sctp@vger.kernel.org, davem@davemloft.net,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	syzkaller@googlegroups.com
Subject: Re: [PATCH net] sctp: not copy sctp_sock pd_lobby in sctp_copy_descendant
Date: Tue, 19 Mar 2019 11:13:00 +0000	[thread overview]
Message-ID: <20190319111300.GB3722@hmswarspite.think-freely.org> (raw)
In-Reply-To: <130ce0bbbc015f9fb47f97b51c650843e2ac39a3.1552910309.git.lucien.xin@gmail.com>

On Mon, Mar 18, 2019 at 07:58:29PM +0800, Xin Long wrote:
> Now sctp_copy_descendant() copies pd_lobby from old sctp scok to new
> sctp sock. If sctp_sock_migrate() returns error, it will panic when
> releasing new sock and trying to purge pd_lobby due to the incorrect
> pointers in pd_lobby.
> 
>   [  120.485116] kasan: CONFIG_KASAN_INLINE enabled
>   [  120.486270] kasan: GPF could be caused by NULL-ptr deref or user
>   [  120.509901] Call Trace:
>   [  120.510443]  sctp_ulpevent_free+0x1e8/0x490 [sctp]
>   [  120.511438]  sctp_queue_purge_ulpevents+0x97/0xe0 [sctp]
>   [  120.512535]  sctp_close+0x13a/0x700 [sctp]
>   [  120.517483]  inet_release+0xdc/0x1c0
>   [  120.518215]  __sock_release+0x1d2/0x2a0
>   [  120.519025]  sctp_do_peeloff+0x30f/0x3c0 [sctp]
> 
> We fix it by not copying sctp_sock pd_lobby in sctp_copy_descendan(),
> and skb_queue_head_init() can also be removed in sctp_sock_migrate().
> 
> Reported-by: syzbot+85e0b422ff140b03672a@syzkaller.appspotmail.com
> Fixes: 89664c623617 ("sctp: sctp_sock_migrate() returns error if sctp_bind_addr_dup() fails")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/socket.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 6140471..65b5386 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -9169,7 +9169,7 @@ static inline void sctp_copy_descendant(struct sock *sk_to,
>  {
>  	int ancestor_size = sizeof(struct inet_sock) +
>  			    sizeof(struct sctp_sock) -
> -			    offsetof(struct sctp_sock, auto_asconf_list);
> +			    offsetof(struct sctp_sock, pd_lobby);
>  
>  	if (sk_from->sk_family = PF_INET6)
>  		ancestor_size += sizeof(struct ipv6_pinfo);
> @@ -9253,7 +9253,6 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
>  	 * 2) Peeling off partial delivery; keep pd_lobby in new pd_lobby.
>  	 * 3) Peeling off non-partial delivery; move pd_lobby to receive_queue.
>  	 */
> -	skb_queue_head_init(&newsp->pd_lobby);
>  	atomic_set(&sctp_sk(newsk)->pd_mode, assoc->ulpq.pd_mode);
>  
>  	if (atomic_read(&sctp_sk(oldsk)->pd_mode)) {
> -- 
> 2.1.0
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

  parent reply	other threads:[~2019-03-19 11:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-18 11:58 [PATCH net] sctp: not copy sctp_sock pd_lobby in sctp_copy_descendant Xin Long
2019-03-18 11:58 ` Xin Long
2019-03-19  1:24 ` David Miller
2019-03-19  1:24   ` David Miller
2019-03-19 11:13 ` Neil Horman [this message]
2019-03-19 11:13   ` Neil Horman

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=20190319111300.GB3722@hmswarspite.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=davem@davemloft.net \
    --cc=linux-sctp@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=syzkaller@googlegroups.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.