netdev - Re: [PATCH bpf-next] libbpf: Print hint about ulimit when getting permission denied error

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191216155336.GA28925@linux.fritz.box>
Date:   Mon, 16 Dec 2019 16:53:36 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next] libbpf: Print hint about ulimit when getting
 permission denied error

On Mon, Dec 16, 2019 at 02:52:30PM +0100, Jesper Dangaard Brouer wrote:
> On Mon, 16 Dec 2019 13:40:31 +0100
> Toke Høiland-Jørgensen <toke@...hat.com> wrote:
> 
> > Probably the single most common error newcomers to XDP are stumped by is
> > the 'permission denied' error they get when trying to load their program
> > and 'ulimit -r' is set too low. For examples, see [0], [1].
> > 
> > Since the error code is UAPI, we can't change that. Instead, this patch
> > adds a few heuristics in libbpf and outputs an additional hint if they are
> > met: If an EPERM is returned on map create or program load, and geteuid()
> > shows we are root, and the current RLIMIT_MEMLOCK is not infinity, we
> > output a hint about raising 'ulimit -r' as an additional log line.
> > 
> > [0] https://marc.info/?l=xdp-newbies&m=157043612505624&w=2
> > [1] https://github.com/xdp-project/xdp-tutorial/issues/86
> > 
> > Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> 
> Acked-by: Jesper Dangaard Brouer <brouer@...hat.com>
> 
> This is the top #1 issue users hit again-and-again, too bad we cannot
> change the return code as it is UAPI now.  Thanks for taking care of
> this mitigation.

It's an annoying error that comes up very often, agree, and tooling then
sets it to a high value / inf anyway as next step if it has the rights
to do so. Probably time to revisit the idea that if the user has the same
rights as being able to set setrlimit() anyway, we should just not account
for it ... incomplete hack:

 kernel/bpf/syscall.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b08c362f4e02..116581c32848 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -203,12 +203,17 @@ void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr)

 static int bpf_charge_memlock(struct user_struct *user, u32 pages)
 {
-	unsigned long memlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+	unsigned long memlock_limit;

+	if (capable(CAP_SYS_RESOURCE))
+		return 0;
+
+	memlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
 	if (atomic_long_add_return(pages, &user->locked_vm) > memlock_limit) {
 		atomic_long_sub(pages, &user->locked_vm);
 		return -EPERM;
 	}
+
 	return 0;
 }

@@ -1339,12 +1344,12 @@ static void bpf_audit_prog(const struct bpf_prog *prog, unsigned int op)

 int __bpf_prog_charge(struct user_struct *user, u32 pages)
 {
-	unsigned long memlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
-	unsigned long user_bufs;
+	unsigned long memlock_limit;

-	if (user) {
-		user_bufs = atomic_long_add_return(pages, &user->locked_vm);
-		if (user_bufs > memlock_limit) {
+	if (user && !capable(CAP_SYS_RESOURCE)) {
+		memlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+		if (atomic_long_add_return(pages, &user->locked_vm) >
+		    memlock_limit) {
 			atomic_long_sub(pages, &user->locked_vm);
 			return -EPERM;
 		}
--
2.21.0