Re: [OE-core] [gatesgarth][PATCH 13/22] bluez5: fix CVE-2020-27153

["Anuj Mittal" <anuj.mittal@intel.com>]

On Fri, 2020-11-06 at 05:12 -1000, Steve Sakoman wrote:
> This morning I also submitted a patch to fix CVE-2020-27153 in
> dunfell
> (bluez5: update to 5.55 to fix CVE-2020-27153):
> 
> https://lists.openembedded.org/g/openembedded-core/message/144343
> 
> 5.55 seems to be a security/bug fix release so it seemed appropriate:
> 
> https://github.com/bluez/bluez/commit/5a180f2ec9edfacafd95e5fed20d36fe8e077f07
> 
> We should do the same fix in dunfell/gatesgarth, so I'd love to get
> some feedback from the community on the preferred approach.

I will drop the CVE patch from my request and take the version upgrade
too. I think we don't usually upgrade bluez in stable branch and other
LTS distros have the same policy. Even though this version has more
than 350 commits, it does seem to be a bug fix release so should be
okay I guess ...

Thanks,

Anuj

> 
> Steve
> 
> On Thu, Nov 5, 2020 at 8:28 PM Anuj Mittal <anuj.mittal@intel.com>
> wrote:
> > 
> > From: Chee Yang Lee <chee.yang.lee@intel.com>
> > 
> > (From OE-Core rev: 4b0688bb8abb2fb8a620541207d40e90e4bf16f9)
> > 
> > Signed-off-by: Chee Yang Lee <chee.yang.lee@intel.com>
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  .../bluez5/bluez5/CVE-2020-27153.patch        | 146
> > ++++++++++++++++++
> >  .../bluez5/bluez5_5.54.bb                     |   2 +
> >  2 files changed, 148 insertions(+)
> >  create mode 100644 meta/recipes-connectivity/bluez5/bluez5/CVE-
> > 2020-27153.patch
> > 
> > diff --git a/meta/recipes-connectivity/bluez5/bluez5/CVE-2020-
> > 27153.patch b/meta/recipes-connectivity/bluez5/bluez5/CVE-2020-
> > 27153.patch
> > new file mode 100644
> > index 0000000000..7b06dd2071
> > --- /dev/null
> > +++ b/meta/recipes-connectivity/bluez5/bluez5/CVE-2020-27153.patch
> > @@ -0,0 +1,146 @@
> > +From 1cd644db8c23a2f530ddb93cebed7dacc5f5721a Mon Sep 17 00:00:00
> > 2001
> > +From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > +Date: Wed, 15 Jul 2020 18:25:37 -0700
> > +Subject: [PATCH] shared/att: Fix possible crash on disconnect
> > +
> > +If there are pending request while disconnecting they would be
> > notified
> > +but clients may endup being freed in the proccess which will then
> > be
> > +calling bt_att_cancel to cancal its requests causing the following
> > +trace:
> > +
> > +Invalid read of size 4
> > +   at 0x1D894C: enable_ccc_callback (gatt-client.c:1627)
> > +   by 0x1D247B: disc_att_send_op (att.c:417)
> > +   by 0x1CCC17: queue_remove_all (queue.c:354)
> > +   by 0x1D47B7: disconnect_cb (att.c:635)
> > +   by 0x1E0707: watch_callback (io-glib.c:170)
> > +   by 0x48E963B: g_main_context_dispatch (in /usr/lib/libglib-
> > 2.0.so.0.6400.4)
> > +   by 0x48E9AC7: ??? (in /usr/lib/libglib-2.0.so.0.6400.4)
> > +   by 0x48E9ECF: g_main_loop_run (in /usr/lib/libglib-
> > 2.0.so.0.6400.4)
> > +   by 0x1E0E97: mainloop_run (mainloop-glib.c:79)
> > +   by 0x1E13B3: mainloop_run_with_signal (mainloop-notify.c:201)
> > +   by 0x12BC3B: main (main.c:770)
> > + Address 0x7d40a28 is 24 bytes inside a block of size 32 free'd
> > +   at 0x484A2E0: free (vg_replace_malloc.c:540)
> > +   by 0x1CCC17: queue_remove_all (queue.c:354)
> > +   by 0x1CCC83: queue_destroy (queue.c:73)
> > +   by 0x1D7DD7: bt_gatt_client_free (gatt-client.c:2209)
> > +   by 0x16497B: batt_free (battery.c:77)
> > +   by 0x16497B: batt_remove (battery.c:286)
> > +   by 0x1A0013: service_remove (service.c:176)
> > +   by 0x1A9B7B: device_remove_gatt_service (device.c:3691)
> > +   by 0x1A9B7B: gatt_service_removed (device.c:3805)
> > +   by 0x1CC90B: queue_foreach (queue.c:220)
> > +   by 0x1DE27B: notify_service_changed.isra.0.part.0 (gatt-
> > db.c:369)
> > +   by 0x1DE387: notify_service_changed (gatt-db.c:361)
> > +   by 0x1DE387: gatt_db_service_destroy (gatt-db.c:385)
> > +   by 0x1DE3EF: gatt_db_remove_service (gatt-db.c:519)
> > +   by 0x1D674F: discovery_op_complete (gatt-client.c:388)
> > +   by 0x1D6877: discover_primary_cb (gatt-client.c:1260)
> > +   by 0x1E220B: discovery_op_complete (gatt-helpers.c:628)
> > +   by 0x1E249B: read_by_grp_type_cb (gatt-helpers.c:730)
> > +   by 0x1D247B: disc_att_send_op (att.c:417)
> > +   by 0x1CCC17: queue_remove_all (queue.c:354)
> > +   by 0x1D47B7: disconnect_cb (att.c:635)
> > +
> > +Upstream-Status: Backport
> > +[
> > https://github.com/bluez/bluez/commit/1cd644db8c23a2f530ddb93cebed7dacc5f5721a
> > ]
> > +CVE: CVE-2020-27153
> > +Signed-off-by: Chee Yang Lee <chee.yang.lee@intel.com>
> > +---
> > + src/shared/att.c | 46 ++++++++++++++++++++++++++++++++++++++++---
> > ---
> > + 1 file changed, 40 insertions(+), 6 deletions(-)
> > +
> > +diff --git a/src/shared/att.c b/src/shared/att.c
> > +index ed3af2920..58f23dfcb 100644
> > +--- a/src/shared/att.c
> > ++++ b/src/shared/att.c
> > +@@ -84,6 +84,7 @@ struct bt_att {
> > +       struct queue *req_queue;        /* Queued ATT protocol
> > requests */
> > +       struct queue *ind_queue;        /* Queued ATT protocol
> > indications */
> > +       struct queue *write_queue;      /* Queue of PDUs ready to
> > send */
> > ++      bool in_disc;                   /* Cleanup queues on
> > disconnect_cb */
> > +
> > +       bt_att_timeout_func_t timeout_callback;
> > +       bt_att_destroy_func_t timeout_destroy;
> > +@@ -222,8 +223,10 @@ static void destroy_att_send_op(void *data)
> > +       free(op);
> > + }
> > +
> > +-static void cancel_att_send_op(struct att_send_op *op)
> > ++static void cancel_att_send_op(void *data)
> > + {
> > ++      struct att_send_op *op = data;
> > ++
> > +       if (op->destroy)
> > +               op->destroy(op->user_data);
> > +
> > +@@ -631,11 +634,6 @@ static bool disconnect_cb(struct io *io, void
> > *user_data)
> > +       /* Dettach channel */
> > +       queue_remove(att->chans, chan);
> > +
> > +-      /* Notify request callbacks */
> > +-      queue_remove_all(att->req_queue, NULL, NULL,
> > disc_att_send_op);
> > +-      queue_remove_all(att->ind_queue, NULL, NULL,
> > disc_att_send_op);
> > +-      queue_remove_all(att->write_queue, NULL, NULL,
> > disc_att_send_op);
> > +-
> > +       if (chan->pending_req) {
> > +               disc_att_send_op(chan->pending_req);
> > +               chan->pending_req = NULL;
> > +@@ -654,6 +652,15 @@ static bool disconnect_cb(struct io *io, void
> > *user_data)
> > +
> > +       bt_att_ref(att);
> > +
> > ++      att->in_disc = true;
> > ++
> > ++      /* Notify request callbacks */
> > ++      queue_remove_all(att->req_queue, NULL, NULL,
> > disc_att_send_op);
> > ++      queue_remove_all(att->ind_queue, NULL, NULL,
> > disc_att_send_op);
> > ++      queue_remove_all(att->write_queue, NULL, NULL,
> > disc_att_send_op);
> > ++
> > ++      att->in_disc = false;
> > ++
> > +       queue_foreach(att->disconn_list, disconn_handler,
> > INT_TO_PTR(err));
> > +
> > +       bt_att_unregister_all(att);
> > +@@ -1574,6 +1581,30 @@ bool bt_att_chan_cancel(struct bt_att_chan
> > *chan, unsigned int id)
> > +       return true;
> > + }
> > +
> > ++static bool bt_att_disc_cancel(struct bt_att *att, unsigned int
> > id)
> > ++{
> > ++      struct att_send_op *op;
> > ++
> > ++      op = queue_find(att->req_queue, match_op_id,
> > UINT_TO_PTR(id));
> > ++      if (op)
> > ++              goto done;
> > ++
> > ++      op = queue_find(att->ind_queue, match_op_id,
> > UINT_TO_PTR(id));
> > ++      if (op)
> > ++              goto done;
> > ++
> > ++      op = queue_find(att->write_queue, match_op_id,
> > UINT_TO_PTR(id));
> > ++
> > ++done:
> > ++      if (!op)
> > ++              return false;
> > ++
> > ++      /* Just cancel since disconnect_cb will be cleaning up */
> > ++      cancel_att_send_op(op);
> > ++
> > ++      return true;
> > ++}
> > ++
> > + bool bt_att_cancel(struct bt_att *att, unsigned int id)
> > + {
> > +       const struct queue_entry *entry;
> > +@@ -1591,6 +1622,9 @@ bool bt_att_cancel(struct bt_att *att,
> > unsigned int id)
> > +                       return true;
> > +       }
> > +
> > ++      if (att->in_disc)
> > ++              return bt_att_disc_cancel(att, id);
> > ++
> > +       op = queue_remove_if(att->req_queue, match_op_id,
> > UINT_TO_PTR(id));
> > +       if (op)
> > +               goto done;
> > diff --git a/meta/recipes-connectivity/bluez5/bluez5_5.54.bb
> > b/meta/recipes-connectivity/bluez5/bluez5_5.54.bb
> > index 260eee1402..9a21f14fae 100644
> > --- a/meta/recipes-connectivity/bluez5/bluez5_5.54.bb
> > +++ b/meta/recipes-connectivity/bluez5/bluez5_5.54.bb
> > @@ -1,5 +1,7 @@
> >  require bluez5.inc
> > 
> > +SRC_URI += " file://CVE-2020-27153.patch"
> > +
> >  SRC_URI[md5sum] = "e637feb2dbb7582bbbff1708367a847c"
> >  SRC_URI[sha256sum] =
> > "68cdab9e63e8832b130d5979dc8c96fdb087b31278f342874d992af3e56656dc"
> > 
> > --
> > 2.28.0
> > 
> > 
> > 
> >