[LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Petr Vorel]

To avoid forcing users to run this setup to avoid TCONF:

tst_capability.c:29: TINFO: Dropping CAP_BPF(39)
bpf_common.c:39: TCONF: Hint: check also /proc/sys/kernel/unprivileged_bpf_disabled
bpf_common.c:41: TCONF: bpf() requires CAP_SYS_ADMIN or CAP_BPF on this system: EPERM (1)

Unfortunately this requires running as root.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/bpf/bpf_prog05.c | 5 +++++
 testcases/kernel/syscalls/bpf/bpf_prog06.c | 5 +++++
 testcases/kernel/syscalls/bpf/bpf_prog07.c | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/testcases/kernel/syscalls/bpf/bpf_prog05.c b/testcases/kernel/syscalls/bpf/bpf_prog05.c
index 2be5a2cc9..8197467d9 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog05.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog05.c
@@ -209,6 +209,11 @@ static struct tst_test test = {
 		{&msg, .size = sizeof(MSG)},
 		{}
 	},
+	.needs_root = 1,
+	.save_restore = (const struct tst_path_val[]) {
+		{"?/proc/sys/kernel/unprivileged_bpf_disabled", "0"},
+		{}
+	},
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "f6b1b3bf0d5f"},
 		{"linux-git", "468f6eafa6c4"},
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog06.c b/testcases/kernel/syscalls/bpf/bpf_prog06.c
index c38dd8239..6c4f96740 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog06.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog06.c
@@ -150,6 +150,11 @@ static struct tst_test test = {
 		{&msg, .size = sizeof(MSG)},
 		{}
 	},
+	.needs_root = 1,
+	.save_restore = (const struct tst_path_val[]) {
+		{"?/proc/sys/kernel/unprivileged_bpf_disabled", "0"},
+		{}
+	},
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "64620e0a1e71"},
 		{"CVE", "CVE-2021-4204"},
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog07.c b/testcases/kernel/syscalls/bpf/bpf_prog07.c
index 50ff6eed0..ae389e6ce 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog07.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog07.c
@@ -158,6 +158,11 @@ static struct tst_test test = {
 		{&msg, .size = sizeof(MSG)},
 		{}
 	},
+	.needs_root = 1,
+	.save_restore = (const struct tst_path_val[]) {
+		{"?/proc/sys/kernel/unprivileged_bpf_disabled", "0"},
+		{}
+	},
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "64620e0a1e71"},
 		{"CVE", "CVE-2022-23222"},
-- 
2.37.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Petr Vorel]

> To avoid forcing users to run this setup to avoid TCONF:

> tst_capability.c:29: TINFO: Dropping CAP_BPF(39)
> bpf_common.c:39: TCONF: Hint: check also /proc/sys/kernel/unprivileged_bpf_disabled
> bpf_common.c:41: TCONF: bpf() requires CAP_SYS_ADMIN or CAP_BPF on this system: EPERM (1)

If accepted, maybe I should also remove from bpf_map_create():
tst_res(TCONF, "Hint: check also /proc/sys/kernel/unprivileged_bpf_disabled");

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Cyril Hrubis]

Hi!
> --- a/testcases/kernel/syscalls/bpf/bpf_prog05.c
> +++ b/testcases/kernel/syscalls/bpf/bpf_prog05.c
> @@ -209,6 +209,11 @@ static struct tst_test test = {
>  		{&msg, .size = sizeof(MSG)},
>  		{}
>  	},
> +	.needs_root = 1,
> +	.save_restore = (const struct tst_path_val[]) {
> +		{"?/proc/sys/kernel/unprivileged_bpf_disabled", "0"},
> +		{}
> +	},

If we set needs_root the test would run under root and there is no need
to fiddle with the unprivileged_bpf_disabled at all.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Petr Vorel]

> Hi!
> > --- a/testcases/kernel/syscalls/bpf/bpf_prog05.c
> > +++ b/testcases/kernel/syscalls/bpf/bpf_prog05.c
> > @@ -209,6 +209,11 @@ static struct tst_test test = {
> >  		{&msg, .size = sizeof(MSG)},
> >  		{}
> >  	},
> > +	.needs_root = 1,
> > +	.save_restore = (const struct tst_path_val[]) {
> > +		{"?/proc/sys/kernel/unprivileged_bpf_disabled", "0"},
> > +		{}
> > +	},

> If we set needs_root the test would run under root and there is no need
> to fiddle with the unprivileged_bpf_disabled at all.

I expected that as well, but well, I don't know why, but:

# cat /proc/sys/kernel/unprivileged_bpf_disabled
2

# id
uid=0(root) gid=0(root) groups=0(root)

# ./bpf_prog05
tst_buffers.c:55: TINFO: Test is using guarded buffers
tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
bpf_common.c:16: TINFO: Raising RLIMIT_MEMLOCK to 10485760
tst_capability.c:29: TINFO: Dropping CAP_SYS_ADMIN(21)
tst_capability.c:29: TINFO: Dropping CAP_BPF(39)
bpf_common.c:39: TCONF: Hint: check also /proc/sys/kernel/unprivileged_bpf_disabled
bpf_common.c:40: TCONF: bpf() requires CAP_SYS_ADMIN or CAP_BPF on this system: EPERM (1)

Summary:
passed   0
failed   0
broken   0
skipped  2
warnings 0

I.e. 1 or 2 kernel.unprivileged_bpf_disabled results bpf() returning EPERM for
*all* users including root. 0 allows running again for all users, but we need
root to set it 0 via .save_restore:

tst_sys_conf.c:106: TBROK: Failed to open FILE '/proc/sys/kernel/unprivileged_bpf_disabled' for writing: EACCES (13)

Maybe we could change tst_sys_conf_save() not to write the value if value can be
read and is the same (and not run tst_sys_conf_restore() if value was the same).

That way we would not need to require root if value is the same.

But it'd be nice to have some tag saying: maybe root is needed, depend on sysfs
value...

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Petr Vorel]

> > Hi!
> > > --- a/testcases/kernel/syscalls/bpf/bpf_prog05.c
> > > +++ b/testcases/kernel/syscalls/bpf/bpf_prog05.c
> > > @@ -209,6 +209,11 @@ static struct tst_test test = {
> > >  		{&msg, .size = sizeof(MSG)},
> > >  		{}
> > >  	},
> > > +	.needs_root = 1,
> > > +	.save_restore = (const struct tst_path_val[]) {
> > > +		{"?/proc/sys/kernel/unprivileged_bpf_disabled", "0"},
> > > +		{}
> > > +	},

> > If we set needs_root the test would run under root and there is no need
> > to fiddle with the unprivileged_bpf_disabled at all.

> I expected that as well, but well, I don't know why, but:

> # cat /proc/sys/kernel/unprivileged_bpf_disabled
> 2

> # id
> uid=0(root) gid=0(root) groups=0(root)

> # ./bpf_prog05
> tst_buffers.c:55: TINFO: Test is using guarded buffers
> tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
> bpf_common.c:16: TINFO: Raising RLIMIT_MEMLOCK to 10485760
> tst_capability.c:29: TINFO: Dropping CAP_SYS_ADMIN(21)
> tst_capability.c:29: TINFO: Dropping CAP_BPF(39)
Maybe dropping CAP_BPF() causes that even running root is not enough.

Kind regards,
Petr

> bpf_common.c:39: TCONF: Hint: check also /proc/sys/kernel/unprivileged_bpf_disabled
> bpf_common.c:40: TCONF: bpf() requires CAP_SYS_ADMIN or CAP_BPF on this system: EPERM (1)

> Summary:
> passed   0
> failed   0
> broken   0
> skipped  2
> warnings 0

> I.e. 1 or 2 kernel.unprivileged_bpf_disabled results bpf() returning EPERM for
> *all* users including root. 0 allows running again for all users, but we need
> root to set it 0 via .save_restore:

> tst_sys_conf.c:106: TBROK: Failed to open FILE '/proc/sys/kernel/unprivileged_bpf_disabled' for writing: EACCES (13)

> Maybe we could change tst_sys_conf_save() not to write the value if value can be
> read and is the same (and not run tst_sys_conf_restore() if value was the same).

> That way we would not need to require root if value is the same.

> But it'd be nice to have some tag saying: maybe root is needed, depend on sysfs
> value...

> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Cyril Hrubis]

Hi!
> I expected that as well, but well, I don't know why, but:
> 
> # cat /proc/sys/kernel/unprivileged_bpf_disabled
> 2
> 
> # id
> uid=0(root) gid=0(root) groups=0(root)
> 
> # ./bpf_prog05
> tst_buffers.c:55: TINFO: Test is using guarded buffers
> tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
> bpf_common.c:16: TINFO: Raising RLIMIT_MEMLOCK to 10485760
> tst_capability.c:29: TINFO: Dropping CAP_SYS_ADMIN(21)
> tst_capability.c:29: TINFO: Dropping CAP_BPF(39)
> bpf_common.c:39: TCONF: Hint: check also /proc/sys/kernel/unprivileged_bpf_disabled
> bpf_common.c:40: TCONF: bpf() requires CAP_SYS_ADMIN or CAP_BPF on this system: EPERM (1)
> 
> Summary:
> passed   0
> failed   0
> broken   0
> skipped  2
> warnings 0
> 
> I.e. 1 or 2 kernel.unprivileged_bpf_disabled results bpf() returning EPERM for
> *all* users including root. 0 allows running again for all users, but we need
> root to set it 0 via .save_restore:

Ah, right, these tests test bugs in unpriviledged bpf and drop
priviledges before they start, see the CAP_DROP in the .caps in the
tst_test struct. So obviously we have to enable unprivileged bpf to run
them. So I guess the patch should go in as it is.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Cyril Hrubis]

Hi!
> I.e. 1 or 2 kernel.unprivileged_bpf_disabled results bpf() returning EPERM for
> *all* users including root. 0 allows running again for all users, but we need
> root to set it 0 via .save_restore:
> 
> tst_sys_conf.c:106: TBROK: Failed to open FILE '/proc/sys/kernel/unprivileged_bpf_disabled' for writing: EACCES (13)
> 
> Maybe we could change tst_sys_conf_save() not to write the value if value can be
> read and is the same (and not run tst_sys_conf_restore() if value was the same).

That would be a good idea either way.

The unprivileged_bpf_disabled is more complicated that this though. It's
a three state as:

0 - enabled
1 - disabled and can't be enabled
2 - disabled and can be enabled

So either we add special handling for 'cannot be changed' value to
save_restore or we have to move that code to the test setup and check
it manually.

> That way we would not need to require root if value is the same.
> 
> But it'd be nice to have some tag saying: maybe root is needed, depend on sysfs
> value...

I wouldn't overly complicate the situation and just require root here.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Petr Vorel]

> Hi!
> > I.e. 1 or 2 kernel.unprivileged_bpf_disabled results bpf() returning EPERM for
> > *all* users including root. 0 allows running again for all users, but we need
> > root to set it 0 via .save_restore:

> > tst_sys_conf.c:106: TBROK: Failed to open FILE '/proc/sys/kernel/unprivileged_bpf_disabled' for writing: EACCES (13)

> > Maybe we could change tst_sys_conf_save() not to write the value if value can be
> > read and is the same (and not run tst_sys_conf_restore() if value was the same).

> That would be a good idea either way.

> The unprivileged_bpf_disabled is more complicated that this though. It's
> a three state as:

> 0 - enabled
> 1 - disabled and can't be enabled
> 2 - disabled and can be enabled
Good point, I didn't realize 1 means "no" also for root :).

> So either we add special handling for 'cannot be changed' value to
> save_restore or we have to move that code to the test setup and check
> it manually.
Yes, because ? check for failure only in tst_sys_conf_save() (saving original
value), but writing new value (0) fails in tst_sys_conf_set() due
SAFE_FILE_PRINTF(). Adding new symbol or changing '?' to to use FILE_PRINTF()
and prints warning would IMHO help. I'll try to send patch soon.

> > That way we would not need to require root if value is the same.

> > But it'd be nice to have some tag saying: maybe root is needed, depend on sysfs
> > value...

> I wouldn't overly complicate the situation and just require root here.
Makes sense.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0

[Richard Palethorpe]

Hello,

Petr Vorel <pvorel@suse.cz> writes:

>> Hi!
>> > I.e. 1 or 2 kernel.unprivileged_bpf_disabled results bpf() returning EPERM for
>> > *all* users including root. 0 allows running again for all users, but we need
>> > root to set it 0 via .save_restore:
>
>> > tst_sys_conf.c:106: TBROK: Failed to open FILE
>> > '/proc/sys/kernel/unprivileged_bpf_disabled' for writing: EACCES
>> > (13)
>
>> > Maybe we could change tst_sys_conf_save() not to write the value if value can be
>> > read and is the same (and not run tst_sys_conf_restore() if value was the same).
>
>> That would be a good idea either way.
>
>> The unprivileged_bpf_disabled is more complicated that this though. It's
>> a three state as:
>
>> 0 - enabled
>> 1 - disabled and can't be enabled
>> 2 - disabled and can be enabled
> Good point, I didn't realize 1 means "no" also for root :).

IMO I've always thought that it's not worth tyring to change this value
because of this and also the hopeless nature of unprivileged eBPF.

OTOH if it is set to 1 then we can argue that known bugs should be fixed
because setting it to 1 shows intent to use it.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp