All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Muchun Song <smuchun@gmail.com>,
	Mukesh Ojha <mojha@codeaurora.org>,
	Prateek Sood <prsood@codeaurora.org>
Subject: [PATCH 4.19 33/50] driver core: Fix use-after-free and double free on glue directory
Date: Wed, 18 Sep 2019 08:19:16 +0200	[thread overview]
Message-ID: <20190918061227.070236885@linuxfoundation.org> (raw)
In-Reply-To: <20190918061223.116178343@linuxfoundation.org>

From: Muchun Song <smuchun@gmail.com>

commit ac43432cb1f5c2950408534987e57c2071e24d8f upstream.

There is a race condition between removing glue directory and adding a new
device under the glue dir. It can be reproduced in following test:

CPU1:                                         CPU2:

device_add()
  get_device_parent()
    class_dir_create_and_add()
      kobject_add_internal()
        create_dir()    // create glue_dir

                                              device_add()
                                                get_device_parent()
                                                  kobject_get() // get glue_dir

device_del()
  cleanup_glue_dir()
    kobject_del(glue_dir)

                                                kobject_add()
                                                  kobject_add_internal()
                                                    create_dir() // in glue_dir
                                                      sysfs_create_dir_ns()
                                                        kernfs_create_dir_ns(sd)

      sysfs_remove_dir() // glue_dir->sd=NULL
      sysfs_put()        // free glue_dir->sd

                                                          // sd is freed
                                                          kernfs_new_node(sd)
                                                            kernfs_get(glue_dir)
                                                            kernfs_add_one()
                                                            kernfs_put()

Before CPU1 remove last child device under glue dir, if CPU2 add a new
device under glue dir, the glue_dir kobject reference count will be
increase to 2 via kobject_get() in get_device_parent(). And CPU2 has
been called kernfs_create_dir_ns(), but not call kernfs_new_node().
Meanwhile, CPU1 call sysfs_remove_dir() and sysfs_put(). This result in
glue_dir->sd is freed and it's reference count will be 0. Then CPU2 call
kernfs_get(glue_dir) will trigger a warning in kernfs_get() and increase
it's reference count to 1. Because glue_dir->sd is freed by CPU1, the next
call kernfs_add_one() by CPU2 will fail(This is also use-after-free)
and call kernfs_put() to decrease reference count. Because the reference
count is decremented to 0, it will also call kmem_cache_free() to free
the glue_dir->sd again. This will result in double free.

In order to avoid this happening, we also should make sure that kernfs_node
for glue_dir is released in CPU1 only when refcount for glue_dir kobj is
1 to fix this race.

The following calltrace is captured in kernel 4.14 with the following patch
applied:

commit 726e41097920 ("drivers: core: Remove glue dirs from sysfs earlier")

--------------------------------------------------------------------------
[    3.633703] WARNING: CPU: 4 PID: 513 at .../fs/kernfs/dir.c:494
                Here is WARN_ON(!atomic_read(&kn->count) in kernfs_get().
....
[    3.633986] Call trace:
[    3.633991]  kernfs_create_dir_ns+0xa8/0xb0
[    3.633994]  sysfs_create_dir_ns+0x54/0xe8
[    3.634001]  kobject_add_internal+0x22c/0x3f0
[    3.634005]  kobject_add+0xe4/0x118
[    3.634011]  device_add+0x200/0x870
[    3.634017]  _request_firmware+0x958/0xc38
[    3.634020]  request_firmware_into_buf+0x4c/0x70
....
[    3.634064] kernel BUG at .../mm/slub.c:294!
                Here is BUG_ON(object == fp) in set_freepointer().
....
[    3.634346] Call trace:
[    3.634351]  kmem_cache_free+0x504/0x6b8
[    3.634355]  kernfs_put+0x14c/0x1d8
[    3.634359]  kernfs_create_dir_ns+0x88/0xb0
[    3.634362]  sysfs_create_dir_ns+0x54/0xe8
[    3.634366]  kobject_add_internal+0x22c/0x3f0
[    3.634370]  kobject_add+0xe4/0x118
[    3.634374]  device_add+0x200/0x870
[    3.634378]  _request_firmware+0x958/0xc38
[    3.634381]  request_firmware_into_buf+0x4c/0x70
--------------------------------------------------------------------------

Fixes: 726e41097920 ("drivers: core: Remove glue dirs from sysfs earlier")
Signed-off-by: Muchun Song <smuchun@gmail.com>
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
Signed-off-by: Prateek Sood <prsood@codeaurora.org>
Link: https://lore.kernel.org/r/20190727032122.24639-1-smuchun@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/base/core.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1648,12 +1648,63 @@ static inline struct kobject *get_glue_d
  */
 static void cleanup_glue_dir(struct device *dev, struct kobject *glue_dir)
 {
+	unsigned int ref;
+
 	/* see if we live in a "glue" directory */
 	if (!live_in_glue_dir(glue_dir, dev))
 		return;
 
 	mutex_lock(&gdp_mutex);
-	if (!kobject_has_children(glue_dir))
+	/**
+	 * There is a race condition between removing glue directory
+	 * and adding a new device under the glue directory.
+	 *
+	 * CPU1:                                         CPU2:
+	 *
+	 * device_add()
+	 *   get_device_parent()
+	 *     class_dir_create_and_add()
+	 *       kobject_add_internal()
+	 *         create_dir()    // create glue_dir
+	 *
+	 *                                               device_add()
+	 *                                                 get_device_parent()
+	 *                                                   kobject_get() // get glue_dir
+	 *
+	 * device_del()
+	 *   cleanup_glue_dir()
+	 *     kobject_del(glue_dir)
+	 *
+	 *                                               kobject_add()
+	 *                                                 kobject_add_internal()
+	 *                                                   create_dir() // in glue_dir
+	 *                                                     sysfs_create_dir_ns()
+	 *                                                       kernfs_create_dir_ns(sd)
+	 *
+	 *       sysfs_remove_dir() // glue_dir->sd=NULL
+	 *       sysfs_put()        // free glue_dir->sd
+	 *
+	 *                                                         // sd is freed
+	 *                                                         kernfs_new_node(sd)
+	 *                                                           kernfs_get(glue_dir)
+	 *                                                           kernfs_add_one()
+	 *                                                           kernfs_put()
+	 *
+	 * Before CPU1 remove last child device under glue dir, if CPU2 add
+	 * a new device under glue dir, the glue_dir kobject reference count
+	 * will be increase to 2 in kobject_get(k). And CPU2 has been called
+	 * kernfs_create_dir_ns(). Meanwhile, CPU1 call sysfs_remove_dir()
+	 * and sysfs_put(). This result in glue_dir->sd is freed.
+	 *
+	 * Then the CPU2 will see a stale "empty" but still potentially used
+	 * glue dir around in kernfs_new_node().
+	 *
+	 * In order to avoid this happening, we also should make sure that
+	 * kernfs_node for glue_dir is released in CPU1 only when refcount
+	 * for glue_dir kobj is 1.
+	 */
+	ref = kref_read(&glue_dir->kref);
+	if (!kobject_has_children(glue_dir) && !--ref)
 		kobject_del(glue_dir);
 	kobject_put(glue_dir);
 	mutex_unlock(&gdp_mutex);



  parent reply	other threads:[~2019-09-18  6:24 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-18  6:18 [PATCH 4.19 00/50] 4.19.74-stable review Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 01/50] bridge/mdb: remove wrong use of NLM_F_MULTI Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 02/50] cdc_ether: fix rndis support for Mediatek based smartphones Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 03/50] ipv6: Fix the link time qualifier of ping_v6_proc_exit_net() Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 04/50] isdn/capi: check message length in capi_write() Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 05/50] ixgbe: Fix secpath usage for IPsec TX offload Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 06/50] net: Fix null de-reference of device refcount Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 07/50] net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 08/50] net: phylink: Fix flow control resolution Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 09/50] net: sched: fix reordering issues Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 10/50] sch_hhf: ensure quantum and hhf_non_hh_weight are non-zero Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 11/50] sctp: Fix the link time qualifier of sctp_ctrlsock_exit() Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 12/50] sctp: use transport pf_retrans in sctp_do_8_2_transport_strike Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 13/50] tcp: fix tcp_ecn_withdraw_cwr() to clear TCP_ECN_QUEUE_CWR Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 14/50] tipc: add NULL pointer check before calling kfree_rcu Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 15/50] tun: fix use-after-free when register netdev failed Greg Kroah-Hartman
2019-09-18  6:18 ` [PATCH 4.19 16/50] gpiolib: acpi: Add gpiolib_acpi_run_edge_events_on_boot option and blacklist Greg Kroah-Hartman
2019-09-19  7:46   ` Pavel Machek
2019-09-20 13:56     ` Hans de Goede
2019-09-18  6:19 ` [PATCH 4.19 17/50] gpio: fix line flag validation in linehandle_create Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 18/50] Btrfs: fix assertion failure during fsync and use of stale transaction Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 19/50] ixgbe: Prevent u8 wrapping of ITR value to something less than 10us Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 20/50] genirq: Prevent NULL pointer dereference in resend_irqs() Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 21/50] KVM: s390: kvm_s390_vm_start_migration: check dirty_bitmap before using it as target for memset() Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 22/50] KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 23/50] KVM: x86: work around leak of uninitialized stack contents Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 24/50] KVM: nVMX: handle page fault in vmread Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 25/50] x86/purgatory: Change compiler flags from -mcmodel=kernel to -mcmodel=large to fix kexec relocation errors Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 26/50] powerpc: Add barrier_nospec to raw_copy_in_user() Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 27/50] drm/meson: Add support for XBGR8888 & ABGR8888 formats Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 28/50] clk: rockchip: Dont yell about bad mmc phases when getting Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 29/50] mtd: rawnand: mtk: Fix wrongly assigned OOB buffer pointer issue Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 30/50] PCI: Always allow probing with driver_override Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 31/50] gpio: fix line flag validation in lineevent_create Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 32/50] ubifs: Correctly use tnc_next() in search_dh_cookie() Greg Kroah-Hartman
2019-09-18  6:19 ` Greg Kroah-Hartman [this message]
2019-09-18  6:19 ` [PATCH 4.19 34/50] crypto: talitos - check AES key size Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 35/50] crypto: talitos - fix CTR alg blocksize Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 36/50] crypto: talitos - check data blocksize in ablkcipher Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 37/50] crypto: talitos - fix ECB algs ivsize Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 38/50] crypto: talitos - Do not modify req->cryptlen on decryption Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 39/50] crypto: talitos - HMAC SNOOP NO AFEU mode requires SW icv checking Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 40/50] firmware: ti_sci: Always request response from firmware Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 41/50] drm: panel-orientation-quirks: Add extra quirk table entry for GPD MicroPC Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 42/50] drm/mediatek: mtk_drm_drv.c: Add of_node_put() before goto Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 43/50] Revert "Bluetooth: btusb: driver to enable the usb-wakeup feature" Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 44/50] iio: adc: stm32-dfsdm: fix data type Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 45/50] modules: fix BUG when load module with rodata=n Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 46/50] modules: fix compile error if dont have strict module rwx Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 47/50] platform/x86: pmc_atom: Add CB4063 Beckhoff Automation board to critclk_systems DMI table Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 48/50] rsi: fix a double free bug in rsi_91x_deinit() Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 49/50] nvmem: Use the same permissions for eeprom as for nvmem Greg Kroah-Hartman
2019-09-18  6:19 ` [PATCH 4.19 50/50] x86/build: Add -Wnoaddress-of-packed-member to REALMODE_CFLAGS, to silence GCC9 build warning Greg Kroah-Hartman
2019-09-18 11:59 ` [PATCH 4.19 00/50] 4.19.74-stable review kernelci.org bot
2019-09-18 12:59 ` Guenter Roeck
2019-09-18 13:40   ` Greg Kroah-Hartman
2019-09-18 13:51     ` Greg Kroah-Hartman
2019-09-18 16:28 ` Jon Hunter
2019-09-18 16:28   ` Jon Hunter
2019-09-18 19:15 ` Naresh Kamboju
2019-09-18 19:37 ` Guenter Roeck
2019-09-19  6:37   ` Greg Kroah-Hartman
2019-09-19  1:24 ` shuah

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=20190918061227.070236885@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mojha@codeaurora.org \
    --cc=prsood@codeaurora.org \
    --cc=smuchun@gmail.com \
    --cc=stable@vger.kernel.org \
    /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.