All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] sctp: kfree_rcu asoc
@ 2018-11-30 17:36 ` Xin Long
  0 siblings, 0 replies; 8+ messages in thread
From: Xin Long @ 2018-11-30 17:36 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman

In sctp_hash_transport/sctp_epaddr_lookup_transport, it dereferences
a transport's asoc under rcu_read_lock while asoc is freed not after
a grace period, which leads to a use-after-free panic.

This patch fixes it by calling kfree_rcu to make asoc be freed after
a grace period.

Note that only the asoc's memory is delayed to free in the patch, it
won't cause sk to linger longer.

Thanks Neil and Marcelo to make this clear.

Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
Suggested-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h | 2 ++
 net/sctp/associola.c       | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index a11f937..feada35 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2075,6 +2075,8 @@ struct sctp_association {
 
 	__u64 abandoned_unsent[SCTP_PR_INDEX(MAX) + 1];
 	__u64 abandoned_sent[SCTP_PR_INDEX(MAX) + 1];
+
+	struct rcu_head rcu;
 };
 
 
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 6a28b96..3702f48 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -434,7 +434,7 @@ static void sctp_association_destroy(struct sctp_association *asoc)
 
 	WARN_ON(atomic_read(&asoc->rmem_alloc));
 
-	kfree(asoc);
+	kfree_rcu(asoc, rcu);
 	SCTP_DBG_OBJCNT_DEC(assoc);
 }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net] sctp: kfree_rcu asoc
@ 2018-11-30 17:36 ` Xin Long
  0 siblings, 0 replies; 8+ messages in thread
From: Xin Long @ 2018-11-30 17:36 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: davem, Marcelo Ricardo Leitner, Neil Horman

In sctp_hash_transport/sctp_epaddr_lookup_transport, it dereferences
a transport's asoc under rcu_read_lock while asoc is freed not after
a grace period, which leads to a use-after-free panic.

This patch fixes it by calling kfree_rcu to make asoc be freed after
a grace period.

Note that only the asoc's memory is delayed to free in the patch, it
won't cause sk to linger longer.

Thanks Neil and Marcelo to make this clear.

Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
Suggested-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h | 2 ++
 net/sctp/associola.c       | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index a11f937..feada35 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2075,6 +2075,8 @@ struct sctp_association {
 
 	__u64 abandoned_unsent[SCTP_PR_INDEX(MAX) + 1];
 	__u64 abandoned_sent[SCTP_PR_INDEX(MAX) + 1];
+
+	struct rcu_head rcu;
 };
 
 
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 6a28b96..3702f48 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -434,7 +434,7 @@ static void sctp_association_destroy(struct sctp_association *asoc)
 
 	WARN_ON(atomic_read(&asoc->rmem_alloc));
 
-	kfree(asoc);
+	kfree_rcu(asoc, rcu);
 	SCTP_DBG_OBJCNT_DEC(assoc);
 }
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net] sctp: kfree_rcu asoc
  2018-11-30 17:36 ` Xin Long
@ 2018-11-30 17:50   ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-11-30 17:50 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman

On Sat, Dec 01, 2018 at 01:36:59AM +0800, Xin Long wrote:
> In sctp_hash_transport/sctp_epaddr_lookup_transport, it dereferences
> a transport's asoc under rcu_read_lock while asoc is freed not after
> a grace period, which leads to a use-after-free panic.
> 
> This patch fixes it by calling kfree_rcu to make asoc be freed after
> a grace period.
> 
> Note that only the asoc's memory is delayed to free in the patch, it
> won't cause sk to linger longer.
> 
> Thanks Neil and Marcelo to make this clear.
> 
> Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
> Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
> Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
> Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
> Suggested-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Phew :-)

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/net/sctp/structs.h | 2 ++
>  net/sctp/associola.c       | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index a11f937..feada35 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -2075,6 +2075,8 @@ struct sctp_association {
>  
>  	__u64 abandoned_unsent[SCTP_PR_INDEX(MAX) + 1];
>  	__u64 abandoned_sent[SCTP_PR_INDEX(MAX) + 1];
> +
> +	struct rcu_head rcu;
>  };
>  
>  
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 6a28b96..3702f48 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -434,7 +434,7 @@ static void sctp_association_destroy(struct sctp_association *asoc)
>  
>  	WARN_ON(atomic_read(&asoc->rmem_alloc));
>  
> -	kfree(asoc);
> +	kfree_rcu(asoc, rcu);
>  	SCTP_DBG_OBJCNT_DEC(assoc);
>  }
>  
> -- 
> 2.1.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net] sctp: kfree_rcu asoc
@ 2018-11-30 17:50   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-11-30 17:50 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman

On Sat, Dec 01, 2018 at 01:36:59AM +0800, Xin Long wrote:
> In sctp_hash_transport/sctp_epaddr_lookup_transport, it dereferences
> a transport's asoc under rcu_read_lock while asoc is freed not after
> a grace period, which leads to a use-after-free panic.
> 
> This patch fixes it by calling kfree_rcu to make asoc be freed after
> a grace period.
> 
> Note that only the asoc's memory is delayed to free in the patch, it
> won't cause sk to linger longer.
> 
> Thanks Neil and Marcelo to make this clear.
> 
> Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
> Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
> Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
> Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
> Suggested-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Phew :-)

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/net/sctp/structs.h | 2 ++
>  net/sctp/associola.c       | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index a11f937..feada35 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -2075,6 +2075,8 @@ struct sctp_association {
>  
>  	__u64 abandoned_unsent[SCTP_PR_INDEX(MAX) + 1];
>  	__u64 abandoned_sent[SCTP_PR_INDEX(MAX) + 1];
> +
> +	struct rcu_head rcu;
>  };
>  
>  
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 6a28b96..3702f48 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -434,7 +434,7 @@ static void sctp_association_destroy(struct sctp_association *asoc)
>  
>  	WARN_ON(atomic_read(&asoc->rmem_alloc));
>  
> -	kfree(asoc);
> +	kfree_rcu(asoc, rcu);
>  	SCTP_DBG_OBJCNT_DEC(assoc);
>  }
>  
> -- 
> 2.1.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net] sctp: kfree_rcu asoc
  2018-11-30 17:36 ` Xin Long
@ 2018-11-30 18:19   ` Neil Horman
  -1 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2018-11-30 18:19 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner

On Sat, Dec 01, 2018 at 01:36:59AM +0800, Xin Long wrote:
> In sctp_hash_transport/sctp_epaddr_lookup_transport, it dereferences
> a transport's asoc under rcu_read_lock while asoc is freed not after
> a grace period, which leads to a use-after-free panic.
> 
> This patch fixes it by calling kfree_rcu to make asoc be freed after
> a grace period.
> 
> Note that only the asoc's memory is delayed to free in the patch, it
> won't cause sk to linger longer.
> 
> Thanks Neil and Marcelo to make this clear.
> 
Thank you for taking the extra time on this

Acked-by: Neil Horman <nhorman@tuxdriver.com>

> Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
> Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
> Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
> Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
> Suggested-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/net/sctp/structs.h | 2 ++
>  net/sctp/associola.c       | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index a11f937..feada35 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -2075,6 +2075,8 @@ struct sctp_association {
>  
>  	__u64 abandoned_unsent[SCTP_PR_INDEX(MAX) + 1];
>  	__u64 abandoned_sent[SCTP_PR_INDEX(MAX) + 1];
> +
> +	struct rcu_head rcu;
>  };
>  
>  
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 6a28b96..3702f48 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -434,7 +434,7 @@ static void sctp_association_destroy(struct sctp_association *asoc)
>  
>  	WARN_ON(atomic_read(&asoc->rmem_alloc));
>  
> -	kfree(asoc);
> +	kfree_rcu(asoc, rcu);
>  	SCTP_DBG_OBJCNT_DEC(assoc);
>  }
>  
> -- 
> 2.1.0
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net] sctp: kfree_rcu asoc
@ 2018-11-30 18:19   ` Neil Horman
  0 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2018-11-30 18:19 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner

On Sat, Dec 01, 2018 at 01:36:59AM +0800, Xin Long wrote:
> In sctp_hash_transport/sctp_epaddr_lookup_transport, it dereferences
> a transport's asoc under rcu_read_lock while asoc is freed not after
> a grace period, which leads to a use-after-free panic.
> 
> This patch fixes it by calling kfree_rcu to make asoc be freed after
> a grace period.
> 
> Note that only the asoc's memory is delayed to free in the patch, it
> won't cause sk to linger longer.
> 
> Thanks Neil and Marcelo to make this clear.
> 
Thank you for taking the extra time on this

Acked-by: Neil Horman <nhorman@tuxdriver.com>

> Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
> Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
> Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
> Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
> Suggested-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/net/sctp/structs.h | 2 ++
>  net/sctp/associola.c       | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index a11f937..feada35 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -2075,6 +2075,8 @@ struct sctp_association {
>  
>  	__u64 abandoned_unsent[SCTP_PR_INDEX(MAX) + 1];
>  	__u64 abandoned_sent[SCTP_PR_INDEX(MAX) + 1];
> +
> +	struct rcu_head rcu;
>  };
>  
>  
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 6a28b96..3702f48 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -434,7 +434,7 @@ static void sctp_association_destroy(struct sctp_association *asoc)
>  
>  	WARN_ON(atomic_read(&asoc->rmem_alloc));
>  
> -	kfree(asoc);
> +	kfree_rcu(asoc, rcu);
>  	SCTP_DBG_OBJCNT_DEC(assoc);
>  }
>  
> -- 
> 2.1.0
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net] sctp: kfree_rcu asoc
  2018-11-30 17:36 ` Xin Long
@ 2018-12-03 23:55   ` David Miller
  -1 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-12-03 23:55 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman

From: Xin Long <lucien.xin@gmail.com>
Date: Sat,  1 Dec 2018 01:36:59 +0800

> In sctp_hash_transport/sctp_epaddr_lookup_transport, it dereferences
> a transport's asoc under rcu_read_lock while asoc is freed not after
> a grace period, which leads to a use-after-free panic.
> 
> This patch fixes it by calling kfree_rcu to make asoc be freed after
> a grace period.
> 
> Note that only the asoc's memory is delayed to free in the patch, it
> won't cause sk to linger longer.
> 
> Thanks Neil and Marcelo to make this clear.
> 
> Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
> Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
> Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
> Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
> Suggested-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net] sctp: kfree_rcu asoc
@ 2018-12-03 23:55   ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-12-03 23:55 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman

From: Xin Long <lucien.xin@gmail.com>
Date: Sat,  1 Dec 2018 01:36:59 +0800

> In sctp_hash_transport/sctp_epaddr_lookup_transport, it dereferences
> a transport's asoc under rcu_read_lock while asoc is freed not after
> a grace period, which leads to a use-after-free panic.
> 
> This patch fixes it by calling kfree_rcu to make asoc be freed after
> a grace period.
> 
> Note that only the asoc's memory is delayed to free in the patch, it
> won't cause sk to linger longer.
> 
> Thanks Neil and Marcelo to make this clear.
> 
> Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
> Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
> Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
> Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
> Suggested-by: Neil Horman <nhorman@tuxdriver.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-12-03 23:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-30 17:36 [PATCH net] sctp: kfree_rcu asoc Xin Long
2018-11-30 17:36 ` Xin Long
2018-11-30 17:50 ` Marcelo Ricardo Leitner
2018-11-30 17:50   ` Marcelo Ricardo Leitner
2018-11-30 18:19 ` Neil Horman
2018-11-30 18:19   ` Neil Horman
2018-12-03 23:55 ` David Miller
2018-12-03 23:55   ` David Miller

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.