linux-kernel - Re: [PATCH 01/22] dma-debug: move initialization to common code

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: <294a1469-01d6-10fb-5de1-b1d4513a066c@arm.com>
Date:   Fri, 20 Apr 2018 11:23:43 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Christoph Hellwig <hch@....de>
Cc:     linux-arch@...r.kernel.org, linux-xtensa@...ux-xtensa.org,
        Michal Simek <monstr@...str.eu>,
        Vincent Chen <deanbo422@...il.com>,
        linux-c6x-dev@...ux-c6x.org, linux-parisc@...r.kernel.org,
        linux-sh@...r.kernel.org, linux-hexagon@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
        openrisc@...ts.librecores.org, Greentime Hu <green.hu@...il.com>,
        linux-alpha@...r.kernel.org, sparclinux@...r.kernel.org,
        nios2-dev@...ts.rocketboards.org,
        linux-snps-arc@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 01/22] dma-debug: move initialization to common code

Hi Christoph,

Nice cleanup! Looks good overall, just a couple of nits.

On 20/04/18 09:02, Christoph Hellwig wrote:
[...]
> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
> index 7f5cdc1e6b29..712a897174e4 100644
> --- a/lib/dma-debug.c
> +++ b/lib/dma-debug.c
> @@ -41,6 +41,11 @@
>   #define HASH_FN_SHIFT   13
>   #define HASH_FN_MASK    (HASH_SIZE - 1)
>   
> +/* allow architectures to override this if absolutely required */
> +#ifndef PREALLOC_DMA_DEBUG_ENTRIES
> +#define PREALLOC_DMA_DEBUG_ENTRIES (1 << 16)
> +#endif
> +
>   enum {
>   	dma_debug_single,
>   	dma_debug_page,
> @@ -1004,18 +1009,16 @@ void dma_debug_add_bus(struct bus_type *bus)
>   	bus_register_notifier(bus, nb);
>   }
>   
> -/*
> - * Let the architectures decide how many entries should be preallocated.
> - */
> -void dma_debug_init(u32 num_entries)
> +static int dma_debug_init(void)
>   {
> +	u32 num_entries;

Maybe initialise it to PREALLOC_DMA_DEBUG_ENTRIES?

>   	int i;
>   
>   	/* Do not use dma_debug_initialized here, since we really want to be
>   	 * called to set dma_debug_initialized
>   	 */
>   	if (global_disable)
> -		return;
> +		return 0;
>   
>   	for (i = 0; i < HASH_SIZE; ++i) {
>   		INIT_LIST_HEAD(&dma_entry_hash[i].list);
> @@ -1026,17 +1029,19 @@ void dma_debug_init(u32 num_entries)
>   		pr_err("DMA-API: error creating debugfs entries - disabling\n");
>   		global_disable = true;
>   
> -		return;
> +		return 0;
>   	}
>   
>   	if (req_entries)
>   		num_entries = req_entries;
> +	else
> +		num_entries = PREALLOC_DMA_DEBUG_ENTRIES;
>   
>   	if (prealloc_memory(num_entries) != 0) {
>   		pr_err("DMA-API: debugging out of memory error - disabled\n");
>   		global_disable = true;
>   
> -		return;
> +		return 0;
>   	}
>   
>   	nr_total_entries = num_free_entries;
> @@ -1044,7 +1049,9 @@ void dma_debug_init(u32 num_entries)
>   	dma_debug_initialized = true;
>   
>   	pr_info("DMA-API: debugging enabled by kernel config\n");
> +	return 0;
>   }
> +core_initcall(dma_debug_init);

I think it's worth noting that for most users this now happens much 
earlier than before. In general that's probably good (e.g. on arm64 it 
should prevent false-positives from the Arm SMMU drivers under ACPI), 
and I can't imagine it's high-risk, but it is a behaviour change.

Robin.

>   
>   static __init int dma_debug_cmdline(char *str)
>   {
>