]> git.ipfire.org Git - thirdparty/kernel/stable-queue.git/commitdiff
5.2-stable patches
authorGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Tue, 27 Aug 2019 07:23:57 +0000 (09:23 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Tue, 27 Aug 2019 07:23:57 +0000 (09:23 +0200)
added patches:
rxrpc-fix-local-endpoint-refcounting.patch
rxrpc-fix-read-after-free-in-rxrpc_queue_local.patch

queue-5.2/rxrpc-fix-local-endpoint-refcounting.patch [new file with mode: 0644]
queue-5.2/rxrpc-fix-read-after-free-in-rxrpc_queue_local.patch [new file with mode: 0644]
queue-5.2/series

diff --git a/queue-5.2/rxrpc-fix-local-endpoint-refcounting.patch b/queue-5.2/rxrpc-fix-local-endpoint-refcounting.patch
new file mode 100644 (file)
index 0000000..0eead2e
--- /dev/null
@@ -0,0 +1,281 @@
+From 730c5fd42c1e3652a065448fd235cb9fafb2bd10 Mon Sep 17 00:00:00 2001
+From: David Howells <dhowells@redhat.com>
+Date: Fri, 9 Aug 2019 15:20:41 +0100
+Subject: rxrpc: Fix local endpoint refcounting
+
+From: David Howells <dhowells@redhat.com>
+
+commit 730c5fd42c1e3652a065448fd235cb9fafb2bd10 upstream.
+
+The object lifetime management on the rxrpc_local struct is broken in that
+the rxrpc_local_processor() function is expected to clean up and remove an
+object - but it may get requeued by packets coming in on the backing UDP
+socket once it starts running.
+
+This may result in the assertion in rxrpc_local_rcu() firing because the
+memory has been scheduled for RCU destruction whilst still queued:
+
+       rxrpc: Assertion failed
+       ------------[ cut here ]------------
+       kernel BUG at net/rxrpc/local_object.c:468!
+
+Note that if the processor comes around before the RCU free function, it
+will just do nothing because ->dead is true.
+
+Fix this by adding a separate refcount to count active users of the
+endpoint that causes the endpoint to be destroyed when it reaches 0.
+
+The original refcount can then be used to refcount objects through the work
+processor and cause the memory to be rcu freed when that reaches 0.
+
+Fixes: 4f95dd78a77e ("rxrpc: Rework local endpoint management")
+Reported-by: syzbot+1e0edc4b8b7494c28450@syzkaller.appspotmail.com
+Signed-off-by: David Howells <dhowells@redhat.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ net/rxrpc/af_rxrpc.c     |    4 +-
+ net/rxrpc/ar-internal.h  |    5 ++
+ net/rxrpc/input.c        |   16 ++++++--
+ net/rxrpc/local_object.c |   86 +++++++++++++++++++++++++++++------------------
+ 4 files changed, 72 insertions(+), 39 deletions(-)
+
+--- a/net/rxrpc/af_rxrpc.c
++++ b/net/rxrpc/af_rxrpc.c
+@@ -193,7 +193,7 @@ static int rxrpc_bind(struct socket *soc
+ service_in_use:
+       write_unlock(&local->services_lock);
+-      rxrpc_put_local(local);
++      rxrpc_unuse_local(local);
+       ret = -EADDRINUSE;
+ error_unlock:
+       release_sock(&rx->sk);
+@@ -901,7 +901,7 @@ static int rxrpc_release_sock(struct soc
+       rxrpc_queue_work(&rxnet->service_conn_reaper);
+       rxrpc_queue_work(&rxnet->client_conn_reaper);
+-      rxrpc_put_local(rx->local);
++      rxrpc_unuse_local(rx->local);
+       rx->local = NULL;
+       key_put(rx->key);
+       rx->key = NULL;
+--- a/net/rxrpc/ar-internal.h
++++ b/net/rxrpc/ar-internal.h
+@@ -254,7 +254,8 @@ struct rxrpc_security {
+  */
+ struct rxrpc_local {
+       struct rcu_head         rcu;
+-      atomic_t                usage;
++      atomic_t                active_users;   /* Number of users of the local endpoint */
++      atomic_t                usage;          /* Number of references to the structure */
+       struct rxrpc_net        *rxnet;         /* The network ns in which this resides */
+       struct list_head        link;
+       struct socket           *socket;        /* my UDP socket */
+@@ -1002,6 +1003,8 @@ struct rxrpc_local *rxrpc_lookup_local(s
+ struct rxrpc_local *rxrpc_get_local(struct rxrpc_local *);
+ struct rxrpc_local *rxrpc_get_local_maybe(struct rxrpc_local *);
+ void rxrpc_put_local(struct rxrpc_local *);
++struct rxrpc_local *rxrpc_use_local(struct rxrpc_local *);
++void rxrpc_unuse_local(struct rxrpc_local *);
+ void rxrpc_queue_local(struct rxrpc_local *);
+ void rxrpc_destroy_all_locals(struct rxrpc_net *);
+--- a/net/rxrpc/input.c
++++ b/net/rxrpc/input.c
+@@ -1108,8 +1108,12 @@ static void rxrpc_post_packet_to_local(s
+ {
+       _enter("%p,%p", local, skb);
+-      skb_queue_tail(&local->event_queue, skb);
+-      rxrpc_queue_local(local);
++      if (rxrpc_get_local_maybe(local)) {
++              skb_queue_tail(&local->event_queue, skb);
++              rxrpc_queue_local(local);
++      } else {
++              rxrpc_free_skb(skb, rxrpc_skb_rx_freed);
++      }
+ }
+ /*
+@@ -1119,8 +1123,12 @@ static void rxrpc_reject_packet(struct r
+ {
+       CHECK_SLAB_OKAY(&local->usage);
+-      skb_queue_tail(&local->reject_queue, skb);
+-      rxrpc_queue_local(local);
++      if (rxrpc_get_local_maybe(local)) {
++              skb_queue_tail(&local->reject_queue, skb);
++              rxrpc_queue_local(local);
++      } else {
++              rxrpc_free_skb(skb, rxrpc_skb_rx_freed);
++      }
+ }
+ /*
+--- a/net/rxrpc/local_object.c
++++ b/net/rxrpc/local_object.c
+@@ -79,6 +79,7 @@ static struct rxrpc_local *rxrpc_alloc_l
+       local = kzalloc(sizeof(struct rxrpc_local), GFP_KERNEL);
+       if (local) {
+               atomic_set(&local->usage, 1);
++              atomic_set(&local->active_users, 1);
+               local->rxnet = rxnet;
+               INIT_LIST_HEAD(&local->link);
+               INIT_WORK(&local->processor, rxrpc_local_processor);
+@@ -266,11 +267,8 @@ struct rxrpc_local *rxrpc_lookup_local(s
+                * bind the transport socket may still fail if we're attempting
+                * to use a local address that the dying object is still using.
+                */
+-              if (!rxrpc_get_local_maybe(local)) {
+-                      cursor = cursor->next;
+-                      list_del_init(&local->link);
++              if (!rxrpc_use_local(local))
+                       break;
+-              }
+               age = "old";
+               goto found;
+@@ -284,7 +282,10 @@ struct rxrpc_local *rxrpc_lookup_local(s
+       if (ret < 0)
+               goto sock_error;
+-      list_add_tail(&local->link, cursor);
++      if (cursor != &rxnet->local_endpoints)
++              list_replace(cursor, &local->link);
++      else
++              list_add_tail(&local->link, cursor);
+       age = "new";
+ found:
+@@ -342,7 +343,8 @@ struct rxrpc_local *rxrpc_get_local_mayb
+ }
+ /*
+- * Queue a local endpoint.
++ * Queue a local endpoint unless it has become unreferenced and pass the
++ * caller's reference to the work item.
+  */
+ void rxrpc_queue_local(struct rxrpc_local *local)
+ {
+@@ -351,15 +353,8 @@ void rxrpc_queue_local(struct rxrpc_loca
+       if (rxrpc_queue_work(&local->processor))
+               trace_rxrpc_local(local, rxrpc_local_queued,
+                                 atomic_read(&local->usage), here);
+-}
+-
+-/*
+- * A local endpoint reached its end of life.
+- */
+-static void __rxrpc_put_local(struct rxrpc_local *local)
+-{
+-      _enter("%d", local->debug_id);
+-      rxrpc_queue_work(&local->processor);
++      else
++              rxrpc_put_local(local);
+ }
+ /*
+@@ -375,11 +370,46 @@ void rxrpc_put_local(struct rxrpc_local
+               trace_rxrpc_local(local, rxrpc_local_put, n, here);
+               if (n == 0)
+-                      __rxrpc_put_local(local);
++                      call_rcu(&local->rcu, rxrpc_local_rcu);
+       }
+ }
+ /*
++ * Start using a local endpoint.
++ */
++struct rxrpc_local *rxrpc_use_local(struct rxrpc_local *local)
++{
++      unsigned int au;
++
++      local = rxrpc_get_local_maybe(local);
++      if (!local)
++              return NULL;
++
++      au = atomic_fetch_add_unless(&local->active_users, 1, 0);
++      if (au == 0) {
++              rxrpc_put_local(local);
++              return NULL;
++      }
++
++      return local;
++}
++
++/*
++ * Cease using a local endpoint.  Once the number of active users reaches 0, we
++ * start the closure of the transport in the work processor.
++ */
++void rxrpc_unuse_local(struct rxrpc_local *local)
++{
++      unsigned int au;
++
++      au = atomic_dec_return(&local->active_users);
++      if (au == 0)
++              rxrpc_queue_local(local);
++      else
++              rxrpc_put_local(local);
++}
++
++/*
+  * Destroy a local endpoint's socket and then hand the record to RCU to dispose
+  * of.
+  *
+@@ -393,16 +423,6 @@ static void rxrpc_local_destroyer(struct
+       _enter("%d", local->debug_id);
+-      /* We can get a race between an incoming call packet queueing the
+-       * processor again and the work processor starting the destruction
+-       * process which will shut down the UDP socket.
+-       */
+-      if (local->dead) {
+-              _leave(" [already dead]");
+-              return;
+-      }
+-      local->dead = true;
+-
+       mutex_lock(&rxnet->local_mutex);
+       list_del_init(&local->link);
+       mutex_unlock(&rxnet->local_mutex);
+@@ -422,13 +442,11 @@ static void rxrpc_local_destroyer(struct
+        */
+       rxrpc_purge_queue(&local->reject_queue);
+       rxrpc_purge_queue(&local->event_queue);
+-
+-      _debug("rcu local %d", local->debug_id);
+-      call_rcu(&local->rcu, rxrpc_local_rcu);
+ }
+ /*
+- * Process events on an endpoint
++ * Process events on an endpoint.  The work item carries a ref which
++ * we must release.
+  */
+ static void rxrpc_local_processor(struct work_struct *work)
+ {
+@@ -441,8 +459,10 @@ static void rxrpc_local_processor(struct
+       do {
+               again = false;
+-              if (atomic_read(&local->usage) == 0)
+-                      return rxrpc_local_destroyer(local);
++              if (atomic_read(&local->active_users) == 0) {
++                      rxrpc_local_destroyer(local);
++                      break;
++              }
+               if (!skb_queue_empty(&local->reject_queue)) {
+                       rxrpc_reject_packets(local);
+@@ -454,6 +474,8 @@ static void rxrpc_local_processor(struct
+                       again = true;
+               }
+       } while (again);
++
++      rxrpc_put_local(local);
+ }
+ /*
diff --git a/queue-5.2/rxrpc-fix-read-after-free-in-rxrpc_queue_local.patch b/queue-5.2/rxrpc-fix-read-after-free-in-rxrpc_queue_local.patch
new file mode 100644 (file)
index 0000000..ab92bfd
--- /dev/null
@@ -0,0 +1,122 @@
+From 06d9532fa6b34f12a6d75711162d47c17c1add72 Mon Sep 17 00:00:00 2001
+From: David Howells <dhowells@redhat.com>
+Date: Tue, 13 Aug 2019 22:26:36 +0100
+Subject: rxrpc: Fix read-after-free in rxrpc_queue_local()
+
+From: David Howells <dhowells@redhat.com>
+
+commit 06d9532fa6b34f12a6d75711162d47c17c1add72 upstream.
+
+rxrpc_queue_local() attempts to queue the local endpoint it is given and
+then, if successful, prints a trace line.  The trace line includes the
+current usage count - but we're not allowed to look at the local endpoint
+at this point as we passed our ref on it to the workqueue.
+
+Fix this by reading the usage count before queuing the work item.
+
+Also fix the reading of local->debug_id for trace lines, which must be done
+with the same consideration as reading the usage count.
+
+Fixes: 09d2bf595db4 ("rxrpc: Add a tracepoint to track rxrpc_local refcounting")
+Reported-by: syzbot+78e71c5bab4f76a6a719@syzkaller.appspotmail.com
+Signed-off-by: David Howells <dhowells@redhat.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ include/trace/events/rxrpc.h |    6 +++---
+ net/rxrpc/local_object.c     |   19 ++++++++++---------
+ 2 files changed, 13 insertions(+), 12 deletions(-)
+
+--- a/include/trace/events/rxrpc.h
++++ b/include/trace/events/rxrpc.h
+@@ -498,10 +498,10 @@ rxrpc_tx_points;
+ #define E_(a, b)      { a, b }
+ TRACE_EVENT(rxrpc_local,
+-          TP_PROTO(struct rxrpc_local *local, enum rxrpc_local_trace op,
++          TP_PROTO(unsigned int local_debug_id, enum rxrpc_local_trace op,
+                    int usage, const void *where),
+-          TP_ARGS(local, op, usage, where),
++          TP_ARGS(local_debug_id, op, usage, where),
+           TP_STRUCT__entry(
+                   __field(unsigned int,       local           )
+@@ -511,7 +511,7 @@ TRACE_EVENT(rxrpc_local,
+                            ),
+           TP_fast_assign(
+-                  __entry->local = local->debug_id;
++                  __entry->local = local_debug_id;
+                   __entry->op = op;
+                   __entry->usage = usage;
+                   __entry->where = where;
+--- a/net/rxrpc/local_object.c
++++ b/net/rxrpc/local_object.c
+@@ -93,7 +93,7 @@ static struct rxrpc_local *rxrpc_alloc_l
+               local->debug_id = atomic_inc_return(&rxrpc_debug_id);
+               memcpy(&local->srx, srx, sizeof(*srx));
+               local->srx.srx_service = 0;
+-              trace_rxrpc_local(local, rxrpc_local_new, 1, NULL);
++              trace_rxrpc_local(local->debug_id, rxrpc_local_new, 1, NULL);
+       }
+       _leave(" = %p", local);
+@@ -321,7 +321,7 @@ struct rxrpc_local *rxrpc_get_local(stru
+       int n;
+       n = atomic_inc_return(&local->usage);
+-      trace_rxrpc_local(local, rxrpc_local_got, n, here);
++      trace_rxrpc_local(local->debug_id, rxrpc_local_got, n, here);
+       return local;
+ }
+@@ -335,7 +335,8 @@ struct rxrpc_local *rxrpc_get_local_mayb
+       if (local) {
+               int n = atomic_fetch_add_unless(&local->usage, 1, 0);
+               if (n > 0)
+-                      trace_rxrpc_local(local, rxrpc_local_got, n + 1, here);
++                      trace_rxrpc_local(local->debug_id, rxrpc_local_got,
++                                        n + 1, here);
+               else
+                       local = NULL;
+       }
+@@ -343,16 +344,16 @@ struct rxrpc_local *rxrpc_get_local_mayb
+ }
+ /*
+- * Queue a local endpoint unless it has become unreferenced and pass the
+- * caller's reference to the work item.
++ * Queue a local endpoint and pass the caller's reference to the work item.
+  */
+ void rxrpc_queue_local(struct rxrpc_local *local)
+ {
+       const void *here = __builtin_return_address(0);
++      unsigned int debug_id = local->debug_id;
++      int n = atomic_read(&local->usage);
+       if (rxrpc_queue_work(&local->processor))
+-              trace_rxrpc_local(local, rxrpc_local_queued,
+-                                atomic_read(&local->usage), here);
++              trace_rxrpc_local(debug_id, rxrpc_local_queued, n, here);
+       else
+               rxrpc_put_local(local);
+ }
+@@ -367,7 +368,7 @@ void rxrpc_put_local(struct rxrpc_local
+       if (local) {
+               n = atomic_dec_return(&local->usage);
+-              trace_rxrpc_local(local, rxrpc_local_put, n, here);
++              trace_rxrpc_local(local->debug_id, rxrpc_local_put, n, here);
+               if (n == 0)
+                       call_rcu(&local->rcu, rxrpc_local_rcu);
+@@ -454,7 +455,7 @@ static void rxrpc_local_processor(struct
+               container_of(work, struct rxrpc_local, processor);
+       bool again;
+-      trace_rxrpc_local(local, rxrpc_local_processing,
++      trace_rxrpc_local(local->debug_id, rxrpc_local_processing,
+                         atomic_read(&local->usage), NULL);
+       do {
index 4c542d9b87e97985e42aa17823e9d4c088eba90d..44192cf38587124849d57308150399bc9e39c0cb 100644 (file)
@@ -158,3 +158,5 @@ io_uring-fix-potential-hang-with-polled-io.patch
 io_uring-don-t-enter-poll-loop-if-we-have-cqes-pendi.patch
 io_uring-add-need_resched-check-in-inner-poll-loop.patch
 powerpc-allow-flush_-inval_-dcache_range-to-work-across-ranges-4gb.patch
+rxrpc-fix-local-endpoint-refcounting.patch
+rxrpc-fix-read-after-free-in-rxrpc_queue_local.patch