linux-kernel.vger.kernel.org archive mirror
 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,
	syzbot+de271708674e2093097b@syzkaller.appspotmail.com,
	Shuah Khan <skhan@linuxfoundation.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Anirudh Rayabharam <mail@anirudhrb.com>
Subject: [PATCH 4.19 26/54] firmware_loader: fix use-after-free in firmware_fallback_sysfs
Date: Tue, 10 Aug 2021 19:30:20 +0200	[thread overview]
Message-ID: <20210810172945.037293184@linuxfoundation.org> (raw)
In-Reply-To: <20210810172944.179901509@linuxfoundation.org>

From: Anirudh Rayabharam <mail@anirudhrb.com>

commit 75d95e2e39b27f733f21e6668af1c9893a97de5e upstream.

This use-after-free happens when a fw_priv object has been freed but
hasn't been removed from the pending list (pending_fw_head). The next
time fw_load_sysfs_fallback tries to insert into the list, it ends up
accessing the pending_list member of the previously freed fw_priv.

The root cause here is that all code paths that abort the fw load
don't delete it from the pending list. For example:

        _request_firmware()
          -> fw_abort_batch_reqs()
              -> fw_state_aborted()

To fix this, delete the fw_priv from the list in __fw_set_state() if
the new state is DONE or ABORTED. This way, all aborts will remove
the fw_priv from the list. Accordingly, remove calls to list_del_init
that were being made before calling fw_state_(aborted|done).

Also, in fw_load_sysfs_fallback, don't add the fw_priv to the pending
list if it is already aborted. Instead, just jump out and return early.

Fixes: bcfbd3523f3c ("firmware: fix a double abort case with fw_load_sysfs_fallback")
Cc: stable <stable@vger.kernel.org>
Reported-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
Tested-by: syzbot+de271708674e2093097b@syzkaller.appspotmail.com
Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Anirudh Rayabharam <mail@anirudhrb.com>
Link: https://lore.kernel.org/r/20210728085107.4141-3-mail@anirudhrb.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/firmware_loader/fallback.c |   12 ++++++++----
 drivers/base/firmware_loader/firmware.h |   10 +++++++++-
 drivers/base/firmware_loader/main.c     |    2 ++
 3 files changed, 19 insertions(+), 5 deletions(-)

--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -86,12 +86,11 @@ static void __fw_load_abort(struct fw_pr
 {
 	/*
 	 * There is a small window in which user can write to 'loading'
-	 * between loading done and disappearance of 'loading'
+	 * between loading done/aborted and disappearance of 'loading'
 	 */
-	if (fw_sysfs_done(fw_priv))
+	if (fw_state_is_aborted(fw_priv) || fw_sysfs_done(fw_priv))
 		return;
 
-	list_del_init(&fw_priv->pending_list);
 	fw_state_aborted(fw_priv);
 }
 
@@ -297,7 +296,6 @@ static ssize_t firmware_loading_store(st
 			 * Same logic as fw_load_abort, only the DONE bit
 			 * is ignored and we set ABORT only on failure.
 			 */
-			list_del_init(&fw_priv->pending_list);
 			if (rc) {
 				fw_state_aborted(fw_priv);
 				written = rc;
@@ -559,6 +557,11 @@ static int fw_load_sysfs_fallback(struct
 	}
 
 	mutex_lock(&fw_lock);
+	if (fw_state_is_aborted(fw_priv)) {
+		mutex_unlock(&fw_lock);
+		retval = -EINTR;
+		goto out;
+	}
 	list_add(&fw_priv->pending_list, &pending_fw_head);
 	mutex_unlock(&fw_lock);
 
@@ -584,6 +587,7 @@ static int fw_load_sysfs_fallback(struct
 	} else if (fw_priv->is_paged_buf && !fw_priv->data)
 		retval = -ENOMEM;
 
+out:
 	device_del(f_dev);
 err_put_dev:
 	put_device(f_dev);
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -106,8 +106,16 @@ static inline void __fw_state_set(struct
 
 	WRITE_ONCE(fw_st->status, status);
 
-	if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED)
+	if (status == FW_STATUS_DONE || status == FW_STATUS_ABORTED) {
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+		/*
+		 * Doing this here ensures that the fw_priv is deleted from
+		 * the pending list in all abort/done paths.
+		 */
+		list_del_init(&fw_priv->pending_list);
+#endif
 		complete_all(&fw_st->completion);
+	}
 }
 
 static inline void fw_state_aborted(struct fw_priv *fw_priv)
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -558,8 +558,10 @@ static void fw_abort_batch_reqs(struct f
 		return;
 
 	fw_priv = fw->priv;
+	mutex_lock(&fw_lock);
 	if (!fw_state_is_aborted(fw_priv))
 		fw_state_aborted(fw_priv);
+	mutex_unlock(&fw_lock);
 }
 
 /* called from request_firmware() and request_firmware_work_func() */



  parent reply	other threads:[~2021-08-10 17:32 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-10 17:29 [PATCH 4.19 00/54] 4.19.203-rc1 review Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 01/54] Revert "ACPICA: Fix memory leak caused by _CID repair function" Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 02/54] ALSA: seq: Fix racy deletion of subscriber Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 03/54] ARM: imx: add missing iounmap() Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 04/54] ARM: dts: colibri-imx6ull: limit SDIO clock to 25MHz Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 05/54] ALSA: usb-audio: fix incorrect clock source setting Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 06/54] clk: stm32f4: fix post divisor setup for I2S/SAI PLLs Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 07/54] omap5-board-common: remove not physically existing vdds_1v8_main fixed-regulator Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 08/54] scsi: sr: Return correct event when media event code is 3 Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 09/54] media: videobuf2-core: dequeue if start_streaming fails Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 10/54] net: natsemi: Fix missing pci_disable_device() in probe and remove Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 11/54] sctp: move the active_key update after sh_keys is added Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 12/54] nfp: update ethtool reporting of pauseframe control Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 13/54] net: ipv6: fix returned variable type in ip6_skb_dst_mtu Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 14/54] mips: Fix non-POSIX regexp Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 15/54] bnx2x: fix an error code in bnx2x_nic_load() Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 16/54] net: pegasus: fix uninit-value in get_interrupt_interval Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 17/54] net: fec: fix use-after-free in fec_drv_remove Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 18/54] net: vxge: fix use-after-free in vxge_device_unregister Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 19/54] blk-iolatency: error out if blk_get_queue() failed in iolatency_set_limit() Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 20/54] Bluetooth: defer cleanup of resources in hci_unregister_dev() Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 21/54] USB: usbtmc: Fix RCU stall warning Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 22/54] USB: serial: option: add Telit FD980 composition 0x1056 Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 23/54] USB: serial: ch341: fix character loss at high transfer rates Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 24/54] USB: serial: ftdi_sio: add device ID for Auto-M3 OP-COM v2 Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 25/54] firmware_loader: use -ETIMEDOUT instead of -EAGAIN in fw_load_sysfs_fallback Greg Kroah-Hartman
2021-08-10 17:30 ` Greg Kroah-Hartman [this message]
2021-08-10 17:30 ` [PATCH 4.19 27/54] ALSA: usb-audio: Add registration quirk for JBL Quantum 600 Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 28/54] usb: gadget: f_hid: added GET_IDLE and SET_IDLE handlers Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 29/54] usb: gadget: f_hid: fixed NULL pointer dereference Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 30/54] usb: gadget: f_hid: idle uses the highest byte for duration Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 31/54] usb: otg-fsm: Fix hrtimer list corruption Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 32/54] scripts/tracing: fix the bug that cant parse raw_trace_func Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 33/54] tracing / histogram: Give calculation hist_fields a size Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 34/54] tracing/histogram: Rename "cpu" to "common_cpu" Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 35/54] optee: Clear stale cache entries during initialization Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 36/54] tee: add tee_shm_alloc_kernel_buf() Greg Kroah-Hartman
2021-08-11  7:24   ` Pavel Machek
2021-08-11 13:34     ` Tyler Hicks
2021-08-12 10:56       ` Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 37/54] staging: rtl8723bs: Fix a resource leak in sd_int_dpc Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 38/54] media: rtl28xxu: fix zero-length control request Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 39/54] pipe: increase minimum default pipe size to 2 pages Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 40/54] ext4: fix potential htree corruption when growing large_dir directories Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 41/54] serial: 8250: Mask out floating 16/32-bit bus bits Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 42/54] MIPS: Malta: Do not byte-swap accesses to the CBUS UART Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 43/54] pcmcia: i82092: fix a null pointer dereference bug Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 44/54] KVM: x86: accept userspace interrupt only if no event is injected Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 45/54] KVM: x86/mmu: Fix per-cpu counter corruption on 32-bit builds Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 46/54] spi: meson-spicc: fix memory leak in meson_spicc_remove Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 47/54] perf/x86/amd: Dont touch the AMD64_EVENTSEL_HOSTONLY bit inside the guest Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 48/54] qmi_wwan: add network device usage statistics for qmimux devices Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 49/54] libata: fix ata_pio_sector for CONFIG_HIGHMEM Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 50/54] reiserfs: add check for root_inode in reiserfs_fill_super Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 51/54] reiserfs: check directory items on read from disk Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 52/54] alpha: Send stop IPI to send to online CPUs Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 53/54] net/qla3xxx: fix schedule while atomic in ql_wait_for_drvr_lock and ql_adapter_reset Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 54/54] ARM: imx: add mmdc ipg clock operation for mmdc Greg Kroah-Hartman
2021-08-11  1:07 ` [PATCH 4.19 00/54] 4.19.203-rc1 review Samuel Zou
2021-08-11  9:42 ` Sudip Mukherjee
2021-08-11 17:13 ` Naresh Kamboju
2021-08-11 19:58 ` Guenter Roeck
2021-08-11 22:22 ` Shuah Khan
2021-08-12 19:20 ` Pavel Machek

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=20210810172945.037293184@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mail@anirudhrb.com \
    --cc=mcgrof@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+de271708674e2093097b@syzkaller.appspotmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).