linux-kernel - Re: [PATCH] dma-debug: prevent an error message from causing runtime problems

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] [day] [month] [year] [list]

Message-ID: <215cd3ba-3f4c-ab74-c59e-f099fa64aaac@arm.com>
Date:   Fri, 10 Sep 2021 13:55:52 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Hamza Mahfooz <someguy@...ective-light.com>,
        linux-kernel@...r.kernel.org
Cc:     Jeremy Linton <jeremy.linton@....com>,
        iommu@...ts.linux-foundation.org, Christoph Hellwig <hch@....de>
Subject: Re: [PATCH] dma-debug: prevent an error message from causing runtime
 problems

On 2021-09-10 13:05, Hamza Mahfooz wrote:
> For some drivers, that call add_dma_entry() from somewhere down the call
> stack.

Nit: strictly, drivers don't call add_dma_entry(). Drivers only call the 
DMA API functions, and it is the DMA API internals which take a detour 
through dma-debug when desired.

> If this error condition is triggered once, it causes the error
> message to spam the kernel's printk buffer

Is that true? It doesn't look like anything in dma-debug itself can 
obviously lead to that; I was assuming that in Jeremy's case it's the 
driver which has managed to do something such that every new mapping 
call it makes ends up hitting the warning. A busy network interface is 
probably more than capable of saturating the kernel log with a print for 
every packet (particularly a great big 100GBE-capable multi-queue thing 
like that one).

> and bring the CPU usage up to
> 100%. Also, since there is at least one driver that is in the mainline
> and suffers from the error condition, it is more useful to WARN_ON() here
> instead of just printing the error message (in hopes that it will make it
> easier for other drivers that suffer from this issue to be spotted).
> 
> Link: https://lkml.kernel.org/r/fd67fbac-64bf-f0ea-01e1-5938ccfab9d0@arm.com
> Reported-by: Jeremy Linton <jeremy.linton@....com>
> Signed-off-by: Hamza Mahfooz <someguy@...ective-light.com>
> ---
>   kernel/dma/debug.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index 6c90c69e5311..d9806689666e 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -567,7 +567,9 @@ static void add_dma_entry(struct dma_debug_entry *entry)
>   		pr_err("cacheline tracking ENOMEM, dma-debug disabled\n");
>   		global_disable = true;
>   	} else if (rc == -EEXIST) {
> -		pr_err("cacheline tracking EEXIST, overlapping mappings aren't supported\n");
> +		WARN_ONCE(1,
> +			  pr_fmt("cacheline tracking EEXIST, overlapping mappings aren't supported\n"
> +			 ));

Unless there's some subtlety I'm missing, it would be better to use 
err_printk() here - not only for consistency of output, but also to tie 
in with dma-debug's existing output-limiting controls.

Robin.

>   	}
>   }
>   
>