All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: John Garry <john.garry@huawei.com>, Jens Axboe <axboe@kernel.dk>,
	Sasha Levin <sashal@kernel.org>,
	linux-ide@vger.kernel.org
Subject: [PATCH AUTOSEL 5.4 24/46] libata: Remove extra scsi_host_put() in ata_scsi_add_hosts()
Date: Thu,  9 Apr 2020 23:48:47 -0400	[thread overview]
Message-ID: <20200410034909.8922-24-sashal@kernel.org> (raw)
In-Reply-To: <20200410034909.8922-1-sashal@kernel.org>

From: John Garry <john.garry@huawei.com>

[ Upstream commit 1d72f7aec3595249dbb83291ccac041a2d676c57 ]

If the call to scsi_add_host_with_dma() in ata_scsi_add_hosts() fails,
then we may get use-after-free KASAN warns:

==================================================================
BUG: KASAN: use-after-free in kobject_put+0x24/0x180
Read of size 1 at addr ffff0026b8c80364 by task swapper/0/1
CPU: 1 PID: 1 Comm: swapper/0 Tainted: G        W         5.6.0-rc3-00004-g5a71b206ea82-dirty #1765
Hardware name: Huawei TaiShan 200 (Model 2280)/BC82AMDD, BIOS 2280-V2 CS V3.B160.01 02/24/2020
Call trace:
dump_backtrace+0x0/0x298
show_stack+0x14/0x20
dump_stack+0x118/0x190
print_address_description.isra.9+0x6c/0x3b8
__kasan_report+0x134/0x23c
kasan_report+0xc/0x18
__asan_load1+0x5c/0x68
kobject_put+0x24/0x180
put_device+0x10/0x20
scsi_host_put+0x10/0x18
ata_devres_release+0x74/0xb0
release_nodes+0x2d0/0x470
devres_release_all+0x50/0x78
really_probe+0x2d4/0x560
driver_probe_device+0x7c/0x148
device_driver_attach+0x94/0xa0
__driver_attach+0xa8/0x110
bus_for_each_dev+0xe8/0x158
driver_attach+0x30/0x40
bus_add_driver+0x220/0x2e0
driver_register+0xbc/0x1d0
__pci_register_driver+0xbc/0xd0
ahci_pci_driver_init+0x20/0x28
do_one_initcall+0xf0/0x608
kernel_init_freeable+0x31c/0x384
kernel_init+0x10/0x118
ret_from_fork+0x10/0x18

Allocated by task 5:
save_stack+0x28/0xc8
__kasan_kmalloc.isra.8+0xbc/0xd8
kasan_kmalloc+0xc/0x18
__kmalloc+0x1a8/0x280
scsi_host_alloc+0x44/0x678
ata_scsi_add_hosts+0x74/0x268
ata_host_register+0x228/0x488
ahci_host_activate+0x1c4/0x2a8
ahci_init_one+0xd18/0x1298
local_pci_probe+0x74/0xf0
work_for_cpu_fn+0x2c/0x48
process_one_work+0x488/0xc08
worker_thread+0x330/0x5d0
kthread+0x1c8/0x1d0
ret_from_fork+0x10/0x18

Freed by task 5:
save_stack+0x28/0xc8
__kasan_slab_free+0x118/0x180
kasan_slab_free+0x10/0x18
slab_free_freelist_hook+0xa4/0x1a0
kfree+0xd4/0x3a0
scsi_host_dev_release+0x100/0x148
device_release+0x7c/0xe0
kobject_put+0xb0/0x180
put_device+0x10/0x20
scsi_host_put+0x10/0x18
ata_scsi_add_hosts+0x210/0x268
ata_host_register+0x228/0x488
ahci_host_activate+0x1c4/0x2a8
ahci_init_one+0xd18/0x1298
local_pci_probe+0x74/0xf0
work_for_cpu_fn+0x2c/0x48
process_one_work+0x488/0xc08
worker_thread+0x330/0x5d0
kthread+0x1c8/0x1d0
ret_from_fork+0x10/0x18

There is also refcount issue, as well:
WARNING: CPU: 1 PID: 1 at lib/refcount.c:28 refcount_warn_saturate+0xf8/0x170

The issue is that we make an erroneous extra call to scsi_host_put()
for that host:

So in ahci_init_one()->ata_host_alloc_pinfo()->ata_host_alloc(), we setup
a device release method - ata_devres_release() - which intends to release
the SCSI hosts:

static void ata_devres_release(struct device *gendev, void *res)
{
	...
	for (i = 0; i < host->n_ports; i++) {
		struct ata_port *ap = host->ports[i];

		if (!ap)
			continue;

		if (ap->scsi_host)
			scsi_host_put(ap->scsi_host);

	}
	...
}

However in the ata_scsi_add_hosts() error path, we also call
scsi_host_put() for the SCSI hosts.

Fix by removing the the scsi_host_put() calls in ata_scsi_add_hosts() and
leave this to ata_devres_release().

Fixes: f31871951b38 ("libata: separate out ata_host_alloc() and ata_host_register()")
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/ata/libata-scsi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 58e09ffe8b9cb..5af34a3201ed2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4553,22 +4553,19 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 		 */
 		shost->max_host_blocked = 1;
 
-		rc = scsi_add_host_with_dma(ap->scsi_host,
-						&ap->tdev, ap->host->dev);
+		rc = scsi_add_host_with_dma(shost, &ap->tdev, ap->host->dev);
 		if (rc)
-			goto err_add;
+			goto err_alloc;
 	}
 
 	return 0;
 
- err_add:
-	scsi_host_put(host->ports[i]->scsi_host);
  err_alloc:
 	while (--i >= 0) {
 		struct Scsi_Host *shost = host->ports[i]->scsi_host;
 
+		/* scsi_host_put() is in ata_devres_release() */
 		scsi_remove_host(shost);
-		scsi_host_put(shost);
 	}
 	return rc;
 }
-- 
2.20.1


  parent reply	other threads:[~2020-04-10  3:56 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-10  3:48 [PATCH AUTOSEL 5.4 01/46] cpufreq: imx6q: Fixes unwanted cpu overclocking on i.MX6ULL Sasha Levin
2020-04-10  3:48 ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 02/46] staging: wilc1000: avoid double unlocking of 'wilc->hif_cs' mutex Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 03/46] media: venus: hfi_parser: Ignore HEVC encoding for V1 Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 04/46] firmware: arm_sdei: fix double-lock on hibernate with shared events Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 05/46] null_blk: Fix the null_add_dev() error path Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 06/46] null_blk: Handle null_add_dev() failures properly Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 07/46] null_blk: fix spurious IO errors after failed past-wp access Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 08/46] media: imx: imx7_mipi_csis: Power off the source when stopping streaming Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 09/46] media: imx: imx7-media-csi: Fix video field handling Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 10/46] xhci: bail out early if driver can't accress host in resume Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 11/46] ACPI: EC: Do not clear boot_ec_is_ecdt in acpi_ec_add() Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 12/46] x86: Don't let pgprot_modify() change the page encryption bit Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 13/46] dma-mapping: Fix dma_pgprot() for unencrypted coherent pages Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 14/46] block: keep bdi->io_pages in sync with max_sectors_kb for stacked devices Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 15/46] debugfs: Check module state before warning in {full/open}_proxy_open() Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 16/46] irqchip/versatile-fpga: Handle chained IRQs properly Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 17/46] time/sched_clock: Expire timer in hardirq context Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 18/46] media: allegro: fix type of gop_length in channel_create message Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 19/46] sched: Avoid scale real weight down to zero Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 20/46] selftests/x86/ptrace_syscall_32: Fix no-vDSO segfault Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 21/46] PCI/switchtec: Fix init_completion race condition with poll_wait() Sasha Levin
     [not found] ` <20200410034909.8922-1-sashal-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2020-04-10  3:48   ` [PATCH AUTOSEL 5.4 22/46] block, bfq: move forward the getting of an extra ref in bfq_bfqq_move Sasha Levin
2020-04-10  3:48     ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 23/46] media: i2c: video-i2c: fix build errors due to 'imply hwmon' Sasha Levin
2020-04-10  3:48 ` Sasha Levin [this message]
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 25/46] pstore/platform: fix potential mem leak if pstore_init_fs failed Sasha Levin
2020-04-10  3:48 ` [Cluster-devel] [PATCH AUTOSEL 5.4 26/46] gfs2: Do log_flush in gfs2_ail_empty_gl even if ail list is empty Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [Cluster-devel] [PATCH AUTOSEL 5.4 27/46] gfs2: Don't demote a glock until its revokes are written Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 28/46] cpufreq: imx6q: fix error handling Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 29/46] x86/boot: Use unsigned comparison for addresses Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 30/46] efi/x86: Ignore the memory attributes table on i386 Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 31/46] genirq/irqdomain: Check pointer in irq_domain_alloc_irqs_hierarchy() Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 32/46] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 33/46] block: Fix use-after-free issue accessing struct io_cq Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 34/46] media: i2c: ov5695: Fix power on and off sequences Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48   ` Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 35/46] usb: dwc3: core: add support for disabling SS instances in park mode Sasha Levin
2020-04-10  3:48 ` [PATCH AUTOSEL 5.4 36/46] irqchip/gic-v4: Provide irq_retrigger to avoid circular locking dependency Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 37/46] md: check arrays is suspended in mddev_detach before call quiesce operations Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 38/46] firmware: fix a double abort case with fw_load_sysfs_fallback Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 39/46] spi: spi-fsl-dspi: Replace interruptible wait queue with a simple completion Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 40/46] locking/lockdep: Avoid recursion in lockdep_count_{for,back}ward_deps() Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 41/46] block, bfq: fix use-after-free in bfq_idle_slice_timer_body Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 42/46] btrfs: hold a ref on the root in btrfs_recover_relocation Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 43/46] btrfs: qgroup: ensure qgroup_rescan_running is only set when the worker is at least queued Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 44/46] btrfs: remove a BUG_ON() from merge_reloc_roots() Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 45/46] btrfs: restart relocate_tree_blocks properly Sasha Levin
2020-04-10  3:49 ` [PATCH AUTOSEL 5.4 46/46] btrfs: track reloc roots based on their commit root bytenr Sasha Levin

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=20200410034909.8922-24-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=john.garry@huawei.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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.