Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1360307 > unrolled thread

[PATCH] ARM: DMA: Fix kzalloc flags in __dma_alloc

Back to article view | Back to linux.kernel

Contents

  [PATCH] ARM: DMA: Fix kzalloc flags in __dma_alloc Alexandre Courbot <acourbot@nvidia.com> - 2016-03-18 03:20 +0100
    Re: [PATCH] ARM: DMA: Fix kzalloc flags in __dma_alloc Rabin Vincent <rabin@rab.in> - 2016-03-18 09:10 +0100
      Re: [PATCH] ARM: DMA: Fix kzalloc flags in __dma_alloc Alexandre Courbot <acourbot@nvidia.com> - 2016-03-18 10:40 +0100

#1360307 — [PATCH] ARM: DMA: Fix kzalloc flags in __dma_alloc

Commit 19e6e5e5392b ("ARM: 8547/1: dma-mapping: store buffer
information") allocates a structure meant for internal buffer management
with the GFP flags of the buffer itself. This can trigger the following
safeguard in the slab/slub allocator:

	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
		pr_emerg("gfp: %u\n", flags & GFP_SLAB_BUG_MASK);
		BUG();
	}

Fix this by allocating the structure with GFP_KERNEL, as it is meant to
be used by the kernel and not for DMA.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 arch/arm/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index deac58d5f1f7..6a19ebe6a461 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -762,7 +762,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
 	if (!mask)
 		return NULL;
 
-	buf = kzalloc(sizeof(*buf), gfp);
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
 	if (!buf)
 		return NULL;
 
-- 
2.7.3

[toc] | [next] | [standalone]

#1360409

On Fri, Mar 18, 2016 at 11:12:26AM +0900, Alexandre Courbot wrote:
> Commit 19e6e5e5392b ("ARM: 8547/1: dma-mapping: store buffer
> information") allocates a structure meant for internal buffer management
> with the GFP flags of the buffer itself. This can trigger the following
> safeguard in the slab/slub allocator:
> 
> 	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
> 		pr_emerg("gfp: %u\n", flags & GFP_SLAB_BUG_MASK);
> 		BUG();
> 	}
> 
> Fix this by allocating the structure with GFP_KERNEL, as it is meant to
> be used by the kernel and not for DMA.

We can't use GFP_KERNEL here.  The caller may have passed in gfp flags
which indicate that we can't sleep, and we need to respect that.  What we can
do is mask out the region specifiers in the gfp flags that we pass to
kzalloc().  This is what the other architectures do in their dma
allocation functions:

arch/mips/cavium-octeon/dma-octeon.c:   gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
arch/mips/loongson64/common/dma-swiotlb.c:      gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
arch/mips/mm/dma-default.c:     gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
arch/mips/netlogic/common/nlm-dma.c:    gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
arch/x86/kernel/pci-dma.c:      *gfp &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);

[toc] | [prev] | [next] | [standalone]

#1360464

On 03/18/2016 05:05 PM, Rabin Vincent wrote:
> On Fri, Mar 18, 2016 at 11:12:26AM +0900, Alexandre Courbot wrote:
>> Commit 19e6e5e5392b ("ARM: 8547/1: dma-mapping: store buffer
>> information") allocates a structure meant for internal buffer management
>> with the GFP flags of the buffer itself. This can trigger the following
>> safeguard in the slab/slub allocator:
>>
>> 	if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
>> 		pr_emerg("gfp: %u\n", flags & GFP_SLAB_BUG_MASK);
>> 		BUG();
>> 	}
>>
>> Fix this by allocating the structure with GFP_KERNEL, as it is meant to
>> be used by the kernel and not for DMA.
>
> We can't use GFP_KERNEL here.  The caller may have passed in gfp flags
> which indicate that we can't sleep, and we need to respect that.  What we can
> do is mask out the region specifiers in the gfp flags that we pass to
> kzalloc().  This is what the other architectures do in their dma
> allocation functions:
>
> arch/mips/cavium-octeon/dma-octeon.c:   gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
> arch/mips/loongson64/common/dma-swiotlb.c:      gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
> arch/mips/mm/dma-default.c:     gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
> arch/mips/netlogic/common/nlm-dma.c:    gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
> arch/x86/kernel/pci-dma.c:      *gfp &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);

Makes sense. Too bad GFP_SLAB_BUG_MASK is private, otherwise we could 
have used it directly.

Your comment reminded me that I have applied the same "fix" to 
__iommu_alloc_buffer() last year:

	if (array_size <= PAGE_SIZE)
		pages = kzalloc(array_size, GFP_KERNEL);

... but that one is followed by this, which exists since 2012:

	else
		pages = vzalloc(array_size);

So I guess we are not too worried about sleeping in that particular 
function.

Anyway, I will fix as you suggested and resend.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web