linux-kernel - Re: [PATCH 5.19 319/365] swiotlb: panic if nslabs is too small

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c49d3b2b-9f5a-4257-9085-f7ac107cff40@oracle.com>
Date:   Tue, 23 Aug 2022 10:25:27 -0700
From:   Dongli Zhang <dongli.zhang@...cle.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Christoph Hellwig <hch@....de>,
        Robin Murphy <robin.murphy@....com>,
        Yu Zhao <yuzhao@...gle.com>
Cc:     stable@...r.kernel.org, Sasha Levin <sashal@...nel.org>,
        linux-kernel@...r.kernel.org, iommu@...ts.linux.dev
Subject: Re: [PATCH 5.19 319/365] swiotlb: panic if nslabs is too small

Adding Robin, Yu and swiotlb list.

Hi Greg,

There is an on-going discussion whether to revert this patch, because it breaks
a corner case in MIPS when many kernel CONFIGs are not enabled (related to PCI
and device). As a result, MIPS pre-allocates only PAGE_SIZE buffer as swiotlb.

https://lore.kernel.org/all/20220820012031.1285979-1-yuzhao@google.com/

However, the core idea of the patch is to panic on purpose if the swiotlb is
configured with <1MB memory, in order to sync with the remap failure handler in
swiotlb_init_remap().

Therefore, I am waiting for suggestion from Christoph whether (1) to revert this
patch, or (2) enforce the restriction to disallow <1MB allocation.

Thank you very much!

Dongli Zhang

On 8/23/22 1:03 AM, Greg Kroah-Hartman wrote:
> From: Dongli Zhang <dongli.zhang@...cle.com>
> 
> [ Upstream commit 0bf28fc40d89b1a3e00d1b79473bad4e9ca20ad1 ]
> 
> Panic on purpose if nslabs is too small, in order to sync with the remap
> retry logic.
> 
> In addition, print the number of bytes for tlb alloc failure.
> 
> Signed-off-by: Dongli Zhang <dongli.zhang@...cle.com>
> Signed-off-by: Christoph Hellwig <hch@....de>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>  kernel/dma/swiotlb.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 5830dce6081b..f5304e2f6a35 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -242,6 +242,9 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
>  	if (swiotlb_force_disable)
>  		return;
>  
> +	if (nslabs < IO_TLB_MIN_SLABS)
> +		panic("%s: nslabs = %lu too small\n", __func__, nslabs);
> +
>  	/*
>  	 * By default allocate the bounce buffer memory from low memory, but
>  	 * allow to pick a location everywhere for hypervisors with guest
> @@ -254,7 +257,8 @@ void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
>  	else
>  		tlb = memblock_alloc_low(bytes, PAGE_SIZE);
>  	if (!tlb) {
> -		pr_warn("%s: failed to allocate tlb structure\n", __func__);
> +		pr_warn("%s: Failed to allocate %zu bytes tlb structure\n",
> +			__func__, bytes);
>  		return;
>  	}
>  
>