* [PATCH net-next] bpf: return errno -ENOMEM when exceeding RLIMIT_MEMLOCK
@ 2017-02-20 15:35 Jesper Dangaard Brouer
2017-02-20 15:57 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Jesper Dangaard Brouer @ 2017-02-20 15:35 UTC (permalink / raw)
To: netdev; +Cc: Daniel Borkmann, Alexei Starovoitov, Jesper Dangaard Brouer
It is confusing users of samples/bpf that exceeding the resource
limits for RLIMIT_MEMLOCK result in an "Operation not permitted"
message. This is due to bpf limits check return -EPERM.
Instead return -ENOMEM, like most other users of this API.
Fixes: aaac3ba95e4c ("bpf: charge user for creation of BPF maps and programs")
Fixes: 6c9059817432 ("bpf: pre-allocate hash map elements")
Fixes: 5ccb071e97fb ("bpf: fix overflow in prog accounting")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
kernel/bpf/syscall.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 08a4d287226b..37387a9b0d46 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -85,7 +85,7 @@ int bpf_map_precharge_memlock(u32 pages)
cur = atomic_long_read(&user->locked_vm);
free_uid(user);
if (cur + pages > memlock_limit)
- return -EPERM;
+ return -ENOMEM;
return 0;
}
@@ -101,7 +101,7 @@ static int bpf_map_charge_memlock(struct bpf_map *map)
if (atomic_long_read(&user->locked_vm) > memlock_limit) {
atomic_long_sub(map->pages, &user->locked_vm);
free_uid(user);
- return -EPERM;
+ return -ENOMEM;
}
map->user = user;
return 0;
@@ -658,7 +658,7 @@ int __bpf_prog_charge(struct user_struct *user, u32 pages)
user_bufs = atomic_long_add_return(pages, &user->locked_vm);
if (user_bufs > memlock_limit) {
atomic_long_sub(pages, &user->locked_vm);
- return -EPERM;
+ return -ENOMEM;
}
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next] bpf: return errno -ENOMEM when exceeding RLIMIT_MEMLOCK
2017-02-20 15:35 [PATCH net-next] bpf: return errno -ENOMEM when exceeding RLIMIT_MEMLOCK Jesper Dangaard Brouer
@ 2017-02-20 15:57 ` Daniel Borkmann
2017-02-20 16:25 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2017-02-20 15:57 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev; +Cc: Daniel Borkmann, Alexei Starovoitov
On 02/20/2017 04:35 PM, Jesper Dangaard Brouer wrote:
> It is confusing users of samples/bpf that exceeding the resource
> limits for RLIMIT_MEMLOCK result in an "Operation not permitted"
> message. This is due to bpf limits check return -EPERM.
>
> Instead return -ENOMEM, like most other users of this API.
>
> Fixes: aaac3ba95e4c ("bpf: charge user for creation of BPF maps and programs")
> Fixes: 6c9059817432 ("bpf: pre-allocate hash map elements")
> Fixes: 5ccb071e97fb ("bpf: fix overflow in prog accounting")
Btw, last one just moves the helper so fixes doesn't really apply
there, but apart from that this is already uapi exposed behavior
like this for ~1.5yrs, so unfortunately too late to change now. I
think the original intention (arguably confusing in this context)
was that user doesn't have (rlimit) permission to allocate this
resource.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] bpf: return errno -ENOMEM when exceeding RLIMIT_MEMLOCK
2017-02-20 15:57 ` Daniel Borkmann
@ 2017-02-20 16:25 ` Jesper Dangaard Brouer
2017-02-21 8:06 ` Alexei Starovoitov
0 siblings, 1 reply; 6+ messages in thread
From: Jesper Dangaard Brouer @ 2017-02-20 16:25 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, Daniel Borkmann, Alexei Starovoitov, brouer
On Mon, 20 Feb 2017 16:57:34 +0100
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 02/20/2017 04:35 PM, Jesper Dangaard Brouer wrote:
> > It is confusing users of samples/bpf that exceeding the resource
> > limits for RLIMIT_MEMLOCK result in an "Operation not permitted"
> > message. This is due to bpf limits check return -EPERM.
> >
> > Instead return -ENOMEM, like most other users of this API.
> >
> > Fixes: aaac3ba95e4c ("bpf: charge user for creation of BPF maps and programs")
> > Fixes: 6c9059817432 ("bpf: pre-allocate hash map elements")
> > Fixes: 5ccb071e97fb ("bpf: fix overflow in prog accounting")
>
> Btw, last one just moves the helper so fixes doesn't really apply
> there, but apart from that this is already uapi exposed behavior
> like this for ~1.5yrs, so unfortunately too late to change now. I
> think the original intention (arguably confusing in this context)
> was that user doesn't have (rlimit) permission to allocate this
> resource.
This is obviously confusing end-users, thus it should be fixed IMHO.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] bpf: return errno -ENOMEM when exceeding RLIMIT_MEMLOCK
2017-02-20 16:25 ` Jesper Dangaard Brouer
@ 2017-02-21 8:06 ` Alexei Starovoitov
2017-02-21 13:00 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2017-02-21 8:06 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Daniel Borkmann, netdev, Daniel Borkmann
On Mon, Feb 20, 2017 at 05:25:58PM +0100, Jesper Dangaard Brouer wrote:
> On Mon, 20 Feb 2017 16:57:34 +0100
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> > On 02/20/2017 04:35 PM, Jesper Dangaard Brouer wrote:
> > > It is confusing users of samples/bpf that exceeding the resource
> > > limits for RLIMIT_MEMLOCK result in an "Operation not permitted"
> > > message. This is due to bpf limits check return -EPERM.
> > >
> > > Instead return -ENOMEM, like most other users of this API.
> > >
> > > Fixes: aaac3ba95e4c ("bpf: charge user for creation of BPF maps and programs")
> > > Fixes: 6c9059817432 ("bpf: pre-allocate hash map elements")
> > > Fixes: 5ccb071e97fb ("bpf: fix overflow in prog accounting")
> >
> > Btw, last one just moves the helper so fixes doesn't really apply
> > there, but apart from that this is already uapi exposed behavior
> > like this for ~1.5yrs, so unfortunately too late to change now. I
> > think the original intention (arguably confusing in this context)
> > was that user doesn't have (rlimit) permission to allocate this
> > resource.
>
> This is obviously confusing end-users, thus it should be fixed IMHO.
I don't think it's confusing and I think EPERM makes
the most sense as return code in such situation.
There is also code in iovisor/bcc that specifically looking
for EPERM to adjust ulimit.
May be it's not documented properly, but that's different story.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] bpf: return errno -ENOMEM when exceeding RLIMIT_MEMLOCK
2017-02-21 8:06 ` Alexei Starovoitov
@ 2017-02-21 13:00 ` Jesper Dangaard Brouer
2017-02-22 7:14 ` Alexei Starovoitov
0 siblings, 1 reply; 6+ messages in thread
From: Jesper Dangaard Brouer @ 2017-02-21 13:00 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Daniel Borkmann, netdev, Daniel Borkmann, brouer
On Tue, 21 Feb 2017 00:06:11 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Mon, Feb 20, 2017 at 05:25:58PM +0100, Jesper Dangaard Brouer wrote:
> > On Mon, 20 Feb 2017 16:57:34 +0100
> > Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > > On 02/20/2017 04:35 PM, Jesper Dangaard Brouer wrote:
> > > > It is confusing users of samples/bpf that exceeding the resource
> > > > limits for RLIMIT_MEMLOCK result in an "Operation not permitted"
> > > > message. This is due to bpf limits check return -EPERM.
> > > >
> > > > Instead return -ENOMEM, like most other users of this API.
> > > >
> > > > Fixes: aaac3ba95e4c ("bpf: charge user for creation of BPF maps and programs")
> > > > Fixes: 6c9059817432 ("bpf: pre-allocate hash map elements")
> > > > Fixes: 5ccb071e97fb ("bpf: fix overflow in prog accounting")
> > >
> > > Btw, last one just moves the helper so fixes doesn't really apply
> > > there, but apart from that this is already uapi exposed behavior
> > > like this for ~1.5yrs, so unfortunately too late to change now. I
> > > think the original intention (arguably confusing in this context)
> > > was that user doesn't have (rlimit) permission to allocate this
> > > resource.
> >
> > This is obviously confusing end-users, thus it should be fixed IMHO.
>
> I don't think it's confusing and I think EPERM makes
> the most sense as return code in such situation.
Most other kernel users return ENOMEM.
> There is also code in iovisor/bcc that specifically looking
> for EPERM to adjust ulimit.
If there is already a program that depend on this, then it is ABI and
we cannot change it... drop this patch.
> May be it's not documented properly, but that's different story.
Documented it here:
https://prototype-kernel.readthedocs.io/en/latest/bpf/troubleshooting.html
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next] bpf: return errno -ENOMEM when exceeding RLIMIT_MEMLOCK
2017-02-21 13:00 ` Jesper Dangaard Brouer
@ 2017-02-22 7:14 ` Alexei Starovoitov
0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2017-02-22 7:14 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Daniel Borkmann, netdev, Daniel Borkmann
On Tue, Feb 21, 2017 at 02:00:13PM +0100, Jesper Dangaard Brouer wrote:
> On Tue, 21 Feb 2017 00:06:11 -0800
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
> > On Mon, Feb 20, 2017 at 05:25:58PM +0100, Jesper Dangaard Brouer wrote:
> > > On Mon, 20 Feb 2017 16:57:34 +0100
> > > Daniel Borkmann <daniel@iogearbox.net> wrote:
> > >
> > > > On 02/20/2017 04:35 PM, Jesper Dangaard Brouer wrote:
> > > > > It is confusing users of samples/bpf that exceeding the resource
> > > > > limits for RLIMIT_MEMLOCK result in an "Operation not permitted"
> > > > > message. This is due to bpf limits check return -EPERM.
> > > > >
> > > > > Instead return -ENOMEM, like most other users of this API.
> > > > >
> > > > > Fixes: aaac3ba95e4c ("bpf: charge user for creation of BPF maps and programs")
> > > > > Fixes: 6c9059817432 ("bpf: pre-allocate hash map elements")
> > > > > Fixes: 5ccb071e97fb ("bpf: fix overflow in prog accounting")
> > > >
> > > > Btw, last one just moves the helper so fixes doesn't really apply
> > > > there, but apart from that this is already uapi exposed behavior
> > > > like this for ~1.5yrs, so unfortunately too late to change now. I
> > > > think the original intention (arguably confusing in this context)
> > > > was that user doesn't have (rlimit) permission to allocate this
> > > > resource.
> > >
> > > This is obviously confusing end-users, thus it should be fixed IMHO.
> >
> > I don't think it's confusing and I think EPERM makes
> > the most sense as return code in such situation.
>
> Most other kernel users return ENOMEM.
the places in the kernel that check for memlock are not fully consistent.
perf does this:
lock_limit = rlimit(RLIMIT_MEMLOCK);
lock_limit >>= PAGE_SHIFT;
locked = vma->vm_mm->pinned_vm + extra;
if ((locked > lock_limit) && perf_paranoid_tracepoint_raw() &&
!capable(CAP_IPC_LOCK)) {
ret = -EPERM;
goto unlock;
}
and in bpf land we got an idea of using memlock limit from perf.
> Documented it here:
> https://prototype-kernel.readthedocs.io/en/latest/bpf/troubleshooting.html
Thanks! Looks good.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-22 7:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-20 15:35 [PATCH net-next] bpf: return errno -ENOMEM when exceeding RLIMIT_MEMLOCK Jesper Dangaard Brouer
2017-02-20 15:57 ` Daniel Borkmann
2017-02-20 16:25 ` Jesper Dangaard Brouer
2017-02-21 8:06 ` Alexei Starovoitov
2017-02-21 13:00 ` Jesper Dangaard Brouer
2017-02-22 7:14 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).