Re: EBPF-triggered WARNING at mm/percpu.c:1361 in v4-14-rc2
On 09/28/2017 04:45 PM, Mark Rutland wrote: On Thu, Sep 28, 2017 at 04:37:46PM +0200, Daniel Borkmann wrote: On 09/28/2017 01:27 PM, Mark Rutland wrote: Hi, While fuzzing v4.14-rc2 with Syzkaller, I found it was possible to trigger the warning at mm/percpu.c:1361, on both arm64 and x86_64. This appears to require increasing RLIMIT_MEMLOCK, so to the best of my knowledge this cannot be triggered by an unprivileged user. I've included example splats for both x86_64 and arm64, along with a C reproducer, inline below. It looks like dev_map_alloc() requests a percpu alloction of 32776 bytes, which is larger than the maximum supported allocation size of 32768 bytes. I wonder if it would make more sense to pr_warn() for sizes that are too large, so that callers don't have to roll their own checks against PCPU_MIN_UNIT_SIZE? Perhaps the pr_warn() should be ratelimited; or could there be an option where we only return NULL, not triggering a warn at all (which would likely be what callers might do anyway when checking against PCPU_MIN_UNIT_SIZE and then bailing out)? Those both make sense to me; checking __GFP_NOWARN should be easy enough. Just to check, do you think that dev_map_alloc() should explicitly test the size against PCPU_MIN_UNIT_SIZE, prior to calling pcpu_alloc()? Looks like there are users of __alloc_percpu_gfp() with __GFP_NOWARN in couple of places already, but __GFP_NOWARN is ignored. Would make sense to support that indeed to avoid throwing the warn and just let the caller bail out when it sees the NULL as usual. In some cases (like the current ones) this makes sense, others probably not too much and a WARN would be preferred way, but __alloc_percpu_gfp() could provide such option to simplify some of the code that pre checks against the limit on PCPU_MIN_UNIT_SIZE before calling the allocator and doesn't throw a WARN either; and most likely such check is just to prevent the user from seeing exactly this splat. Thanks, Daniel
Re: EBPF-triggered WARNING at mm/percpu.c:1361 in v4-14-rc2
Hello, On Thu, Sep 28, 2017 at 03:45:38PM +0100, Mark Rutland wrote: > > Perhaps the pr_warn() should be ratelimited; or could there be an > > option where we only return NULL, not triggering a warn at all (which > > would likely be what callers might do anyway when checking against > > PCPU_MIN_UNIT_SIZE and then bailing out)? > > Those both make sense to me; checking __GFP_NOWARN should be easy > enough. That also makes sense. > Just to check, do you think that dev_map_alloc() should explicitly test > the size against PCPU_MIN_UNIT_SIZE, prior to calling pcpu_alloc()? But let's please not do this. Thanks. -- tejun
Re: EBPF-triggered WARNING at mm/percpu.c:1361 in v4-14-rc2
Hello,
On Thu, Sep 28, 2017 at 12:27:28PM +0100, Mark Rutland wrote:
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 59d44d6..f731c45 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -1355,8 +1355,13 @@ static void __percpu *pcpu_alloc(size_t size, size_t
> align, bool reserved,
> bits = size >> PCPU_MIN_ALLOC_SHIFT;
> bit_align = align >> PCPU_MIN_ALLOC_SHIFT;
>
> - if (unlikely(!size || size > PCPU_MIN_UNIT_SIZE || align > PAGE_SIZE
> ||
> -!is_power_of_2(align))) {
> + if (unlikely(size > PCPU_MIN_UNIT_SIZE)) {
> + pr_warn("cannot allocate pcpu chunk of size %zu (max %zu)\n",
> + size, PCPU_MIN_UNIT_SIZE);
WARN_ONCE() probably is the better choice here. We wanna know who
tries to allocate larger than the supported size and increase the size
limit if warranted.
Thanks.
--
tejun
Re: EBPF-triggered WARNING at mm/percpu.c:1361 in v4-14-rc2
On Thu, Sep 28, 2017 at 04:37:46PM +0200, Daniel Borkmann wrote: > On 09/28/2017 01:27 PM, Mark Rutland wrote: > >Hi, > > > >While fuzzing v4.14-rc2 with Syzkaller, I found it was possible to trigger > >the > >warning at mm/percpu.c:1361, on both arm64 and x86_64. This appears to > >require > >increasing RLIMIT_MEMLOCK, so to the best of my knowledge this cannot be > >triggered by an unprivileged user. > > > >I've included example splats for both x86_64 and arm64, along with a C > >reproducer, inline below. > > > >It looks like dev_map_alloc() requests a percpu alloction of 32776 bytes, > >which > >is larger than the maximum supported allocation size of 32768 bytes. > > > >I wonder if it would make more sense to pr_warn() for sizes that are too > >large, so that callers don't have to roll their own checks against > >PCPU_MIN_UNIT_SIZE? > > Perhaps the pr_warn() should be ratelimited; or could there be an > option where we only return NULL, not triggering a warn at all (which > would likely be what callers might do anyway when checking against > PCPU_MIN_UNIT_SIZE and then bailing out)? Those both make sense to me; checking __GFP_NOWARN should be easy enough. Just to check, do you think that dev_map_alloc() should explicitly test the size against PCPU_MIN_UNIT_SIZE, prior to calling pcpu_alloc()? I can spin both patches if so. Thanks, Mark.
Re: EBPF-triggered WARNING at mm/percpu.c:1361 in v4-14-rc2
On 09/28/2017 01:27 PM, Mark Rutland wrote:
Hi,
While fuzzing v4.14-rc2 with Syzkaller, I found it was possible to trigger the
warning at mm/percpu.c:1361, on both arm64 and x86_64. This appears to require
increasing RLIMIT_MEMLOCK, so to the best of my knowledge this cannot be
triggered by an unprivileged user.
I've included example splats for both x86_64 and arm64, along with a C
reproducer, inline below.
It looks like dev_map_alloc() requests a percpu alloction of 32776 bytes, which
is larger than the maximum supported allocation size of 32768 bytes.
I wonder if it would make more sense to pr_warn() for sizes that are too
large, so that callers don't have to roll their own checks against
PCPU_MIN_UNIT_SIZE?
Perhaps the pr_warn() should be ratelimited; or could there be an
option where we only return NULL, not triggering a warn at all (which
would likely be what callers might do anyway when checking against
PCPU_MIN_UNIT_SIZE and then bailing out)?
e.g. something like:
diff --git a/mm/percpu.c b/mm/percpu.c
index 59d44d6..f731c45 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1355,8 +1355,13 @@ static void __percpu *pcpu_alloc(size_t size, size_t
align, bool reserved,
bits = size >> PCPU_MIN_ALLOC_SHIFT;
bit_align = align >> PCPU_MIN_ALLOC_SHIFT;
- if (unlikely(!size || size > PCPU_MIN_UNIT_SIZE || align > PAGE_SIZE ||
-!is_power_of_2(align))) {
+ if (unlikely(size > PCPU_MIN_UNIT_SIZE)) {
+ pr_warn("cannot allocate pcpu chunk of size %zu (max %zu)\n",
+ size, PCPU_MIN_UNIT_SIZE);
+ return NULL;
+ }
+
+ if (unlikely(!size || align > PAGE_SIZE || !is_power_of_2(align))) {
WARN(true, "illegal size (%zu) or align (%zu) for percpu
allocation\n",
size, align);
return NULL;
Thanks,
Mark.
Example splat(x86_64)
[ 138.144185] illegal size (32776) or align (8) for percpu allocation
[ 138.150452] [ cut here ]
[ 138.155074] WARNING: CPU: 1 PID: 2223 at mm/percpu.c:1361
pcpu_alloc+0x7c/0x5f0
[ 138.162369] Modules linked in:
[ 138.165423] CPU: 1 PID: 2223 Comm: repro Not tainted 4.14.0-rc2 #3
[ 138.171593] Hardware name: LENOVO 7484A3G/LENOVO, BIOS 5CKT54AUS 09/07/2009
[ 138.178543] task: 881b73069980 task.stack: a36f40f9
[ 138.184455] RIP: 0010:pcpu_alloc+0x7c/0x5f0
[ 138.188633] RSP: 0018:a36f40f93e00 EFLAGS: 00010286
[ 138.193853] RAX: 0037 RBX: RCX:
[ 138.200974] RDX: 881b7ec94a40 RSI: 881b7ec8cbb8 RDI: 881b7ec8cbb8
[ 138.208097] RBP: a36f40f93e68 R08: 0001 R09: 02c4
[ 138.215219] R10: 562a577047f0 R11: a10ad7cd R12: 881b73216cc0
[ 138.222343] R13: 0014 R14: 7ffebeed0900 R15: ffea
[ 138.229463] FS: 7fef84a15700() GS:881b7ec8()
knlGS:
[ 138.237538] CS: 0010 DS: ES: CR0: 80050033
[ 138.243274] CR2: 7fef84497ba0 CR3: 0001b3235000 CR4: 000406e0
[ 138.250397] Call Trace:
[ 138.252844] __alloc_percpu+0x10/0x20
[ 138.256508] dev_map_alloc+0x122/0x1b0
[ 138.260255] SyS_bpf+0x8f9/0x10b0
[ 138.263570] ? security_task_setrlimit+0x3e/0x60
[ 138.268184] ? do_prlimit+0xa6/0x1f0
[ 138.271760] entry_SYSCALL_64_fastpath+0x13/0x94
[ 138.276372] RIP: 0033:0x7fef84546259
[ 138.279946] RSP: 002b:7ffebeed09b8 EFLAGS: 0206 ORIG_RAX:
0141
[ 138.287503] RAX: ffda RBX: RCX: 7fef84546259
[ 138.294627] RDX: 0014 RSI: 7ffebeed09d0 RDI:
[ 138.301749] RBP: 562a57704780 R08: 7fef84810cb0 R09: 7ffebeed0ae8
[ 138.308874] R10: 562a577047f0 R11: 0206 R12: 562a577045d0
[ 138.315997] R13: 7ffebeed0ae0 R14: R15:
[ 138.323122] Code: fe 00 10 00 00 77 10 48 8b 4d b8 48 89 c8 48 83 e8 01 48 85 c1
74 1e 48 8b 55 b8 48 8b 75 c0 48 c7 c7 90 5e be a0 e8 40 88 f3 ff <0f> ff 45 31
ed e9 5e 02 00 00 4c 8b 6d c0 49 89 cc 49 c1 ec 02
[ 138.341953] ---[ end trace b6e380365bfb8a36 ]---
Example splat (arm64)
[ 17.287365] illegal size (32776) or align (8) for percpu allocation
[ 17.295347] [ cut here ]
[ 17.297191] WARNING: CPU: 1 PID: 1440 at mm/percpu.c:1361
pcpu_alloc+0x120/0x9f0
[ 17.307723] Kernel panic - not syncing: panic_on_warn set ...
[ 17.307723]
[ 17.311755] CPU: 1 PID: 1440 Comm: repro Not tainted
4.14.0-rc2-1-gd7ad33d #115
[ 17.320675] Hardware name: linux,dummy-virt (DT)
[ 17.323858] Call trace:
[ 17.325246] [] dump_backtrace+0x0/0x558
[ 17.332538] [] show_stack+0x20/0x30
[ 17.340391] [] dump_stack+0x128/0x1a0
[ 17.342081] [] panic+0x250/0x518
[ 17.344096] [] __warn+0x2a4/0x310
[ 17.345654] [] report_bug+0x1d4/0x290
[ 17.348652] []
