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: <59E75F4F.7070706@iogearbox.net>
Date:   Wed, 18 Oct 2017 16:03:59 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Tejun Heo <tj@...nel.org>
CC:     davem@...emloft.net, ast@...nel.org, john.fastabend@...il.com,
        mark.rutland@....com, richard@....at, sp3485@...umbia.edu,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Dennis Zhou <dennisszhou@...il.com>
Subject: Re: [PATCH net 0/3] Fix for BPF devmap percpu allocation splat

On 10/18/2017 03:25 PM, Tejun Heo wrote:
> Hello, Daniel.
>
> (cc'ing Dennis)
>
> On Tue, Oct 17, 2017 at 04:55:51PM +0200, Daniel Borkmann wrote:
>> The set fixes a splat in devmap percpu allocation when we alloc
>> the flush bitmap. Patch 1 is a prerequisite for the fix in patch 2,
>> patch 1 is rather small, so if this could be routed via -net, for
>> example, with Tejun's Ack that would be good. Patch 3 gets rid of
>> remaining PCPU_MIN_UNIT_SIZE checks, which are percpu allocator
>> internals and should not be used.
>>
>> Thanks!
>>
>> Daniel Borkmann (3):
>>    mm, percpu: add support for __GFP_NOWARN flag
>
> This looks fine.

Great, thanks!

>>    bpf: fix splat for illegal devmap percpu allocation
>>    bpf: do not test for PCPU_MIN_UNIT_SIZE before percpu allocations
>
> These look okay too but if it helps percpu allocator can expose the
> maximum size / alignment supported to take out the guessing game too.

At least from BPF side there's right now no infra for exposing
max possible alloc sizes for maps to e.g. user space as indication.
There are few users left in the tree, where it would make sense for
having some helpers though:

   arch/tile/kernel/setup.c:729:   if (size < PCPU_MIN_UNIT_SIZE)
   arch/tile/kernel/setup.c:730:           size = PCPU_MIN_UNIT_SIZE;
   drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:346: unsigned int max = (PCPU_MIN_UNIT_SIZE - sizeof(*pools)) << 3;
   drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:352: /* make sure per cpu pool fits into PCPU_MIN_UNIT_SIZE */
   drivers/scsi/libfc/fc_exch.c:2488:       /* reduce range so per cpu pool fits into PCPU_MIN_UNIT_SIZE pool */
   drivers/scsi/libfc/fc_exch.c:2489:      pool_exch_range = (PCPU_MIN_UNIT_SIZE - sizeof(*pool)) /

> Also, the reason why PCPU_MIN_UNIT_SIZE is what it is is because
> nobody needed anything bigger.  Increasing the size doesn't really
> cost much at least on 64bit archs.  Is that something we want to be
> considering?

For devmap (and cpumap) itself it wouldn't make sense. For per-cpu
hashtable we could indeed consider it in the future.

Thanks,
Daniel

Powered by blists - more mailing lists