Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8838BC433EF for ; Mon, 15 Nov 2021 21:14:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6EDD861AAA for ; Mon, 15 Nov 2021 21:14:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1349483AbhKOVQ5 (ORCPT ); Mon, 15 Nov 2021 16:16:57 -0500 Received: from mail.kernel.org ([198.145.29.99]:58650 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240550AbhKOSPx (ORCPT ); Mon, 15 Nov 2021 13:15:53 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 9F6BA633E2; Mon, 15 Nov 2021 17:49:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1636998582; bh=KqPV+5idkf2+NjD6Q1uow3wjLkwmEutaUPHgxT3W8Fw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Kq9JpYSE0ZUaVQPAeY2Ex39teZ8UqLgXPOwD54l1NqG4q5c8gra+OpHKWC+1X8/gj srJpscRlCbZOZ/TOCdsKyREoazYu2N96HzFJBBjuzKeiGPkbeek/RT09JAM7jhFXN3 HkK4toZCs0ns+Fi9xl8kRbrYWfX20+6NGslN/g1g= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Halil Pasic , bfu@redhat.com, Vineeth Vijayan , Cornelia Huck , Vasily Gorbik Subject: [PATCH 5.10 561/575] s390/cio: make ccw_device_dma_* more robust Date: Mon, 15 Nov 2021 18:04:46 +0100 Message-Id: <20211115165403.093301133@linuxfoundation.org> X-Mailer: git-send-email 2.33.1 In-Reply-To: <20211115165343.579890274@linuxfoundation.org> References: <20211115165343.579890274@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Halil Pasic commit ad9a14517263a16af040598c7920c09ca9670a31 upstream. Since commit 48720ba56891 ("virtio/s390: use DMA memory for ccw I/O and classic notifiers") we were supposed to make sure that virtio_ccw_release_dev() completes before the ccw device and the attached dma pool are torn down, but unfortunately we did not. Before that commit it used to be OK to delay cleaning up the memory allocated by virtio-ccw indefinitely (which isn't really intuitive for guys used to destruction happens in reverse construction order), but now we trigger a BUG_ON if the genpool is destroyed before all memory allocated from it is deallocated. Which brings down the guest. We can observe this problem, when unregister_virtio_device() does not give up the last reference to the virtio_device (e.g. because a virtio-scsi attached scsi disk got removed without previously unmounting its previously mounted partition). To make sure that the genpool is only destroyed after all the necessary freeing is done let us take a reference on the ccw device on each ccw_device_dma_zalloc() and give it up on each ccw_device_dma_free(). Actually there are multiple approaches to fixing the problem at hand that can work. The upside of this one is that it is the safest one while remaining simple. We don't crash the guest even if the driver does not pair allocations and frees. The downside is the reference counting overhead, that the reference counting for ccw devices becomes more complex, in a sense that we need to pair the calls to the aforementioned functions for it to be correct, and that if we happen to leak, we leak more than necessary (the whole ccw device instead of just the genpool). Some alternatives to this approach are taking a reference in virtio_ccw_online() and giving it up in virtio_ccw_release_dev() or making sure virtio_ccw_release_dev() completes its work before virtio_ccw_remove() returns. The downside of these approaches is that these are less safe against programming errors. Cc: # v5.3 Signed-off-by: Halil Pasic Fixes: 48720ba56891 ("virtio/s390: use DMA memory for ccw I/O and classic notifiers") Reported-by: bfu@redhat.com Reviewed-by: Vineeth Vijayan Acked-by: Cornelia Huck Signed-off-by: Vasily Gorbik Signed-off-by: Greg Kroah-Hartman --- drivers/s390/cio/device_ops.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) --- a/drivers/s390/cio/device_ops.c +++ b/drivers/s390/cio/device_ops.c @@ -825,13 +825,23 @@ EXPORT_SYMBOL_GPL(ccw_device_get_chid); */ void *ccw_device_dma_zalloc(struct ccw_device *cdev, size_t size) { - return cio_gp_dma_zalloc(cdev->private->dma_pool, &cdev->dev, size); + void *addr; + + if (!get_device(&cdev->dev)) + return NULL; + addr = cio_gp_dma_zalloc(cdev->private->dma_pool, &cdev->dev, size); + if (IS_ERR_OR_NULL(addr)) + put_device(&cdev->dev); + return addr; } EXPORT_SYMBOL(ccw_device_dma_zalloc); void ccw_device_dma_free(struct ccw_device *cdev, void *cpu_addr, size_t size) { + if (!cpu_addr) + return; cio_gp_dma_free(cdev->private->dma_pool, cpu_addr, size); + put_device(&cdev->dev); } EXPORT_SYMBOL(ccw_device_dma_free);