* [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
@ 2022-08-18 17:17 Petr Vorel
2022-08-18 17:50 ` Petr Vorel
2022-08-19 8:27 ` Cyril Hrubis
0 siblings, 2 replies; 9+ messages in thread
From: Petr Vorel @ 2022-08-18 17:17 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
2022-08-18 17:17 [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0 Petr Vorel
@ 2022-08-18 17:50 ` Petr Vorel
2022-08-19 8:27 ` Cyril Hrubis
1 sibling, 0 replies; 9+ messages in thread
From: Petr Vorel @ 2022-08-18 17:50 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
2022-08-18 17:17 [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0 Petr Vorel
2022-08-18 17:50 ` Petr Vorel
@ 2022-08-19 8:27 ` Cyril Hrubis
2022-08-19 10:28 ` Petr Vorel
1 sibling, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2022-08-19 8:27 UTC (permalink / raw)
To: Petr Vorel; +Cc: Richard Palethorpe, ltp
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
2022-08-19 8:27 ` Cyril Hrubis
@ 2022-08-19 10:28 ` Petr Vorel
2022-08-19 10:31 ` Petr Vorel
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Petr Vorel @ 2022-08-19 10:28 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Richard Palethorpe, ltp
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
2022-08-19 10:28 ` Petr Vorel
@ 2022-08-19 10:31 ` Petr Vorel
2022-08-19 10:43 ` Cyril Hrubis
2022-08-19 11:00 ` Cyril Hrubis
2 siblings, 0 replies; 9+ messages in thread
From: Petr Vorel @ 2022-08-19 10:31 UTC (permalink / raw)
To: Cyril Hrubis, ltp, Richard Palethorpe
> > 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
2022-08-19 10:28 ` Petr Vorel
2022-08-19 10:31 ` Petr Vorel
@ 2022-08-19 10:43 ` Cyril Hrubis
2022-08-19 11:00 ` Cyril Hrubis
2 siblings, 0 replies; 9+ messages in thread
From: Cyril Hrubis @ 2022-08-19 10:43 UTC (permalink / raw)
To: Petr Vorel; +Cc: Richard Palethorpe, ltp
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
2022-08-19 10:28 ` Petr Vorel
2022-08-19 10:31 ` Petr Vorel
2022-08-19 10:43 ` Cyril Hrubis
@ 2022-08-19 11:00 ` Cyril Hrubis
2022-08-19 12:08 ` Petr Vorel
2 siblings, 1 reply; 9+ messages in thread
From: Cyril Hrubis @ 2022-08-19 11:00 UTC (permalink / raw)
To: Petr Vorel; +Cc: Richard Palethorpe, ltp
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
2022-08-19 11:00 ` Cyril Hrubis
@ 2022-08-19 12:08 ` Petr Vorel
2022-08-25 14:22 ` Richard Palethorpe
0 siblings, 1 reply; 9+ messages in thread
From: Petr Vorel @ 2022-08-19 12:08 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Richard Palethorpe, ltp
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0
2022-08-19 12:08 ` Petr Vorel
@ 2022-08-25 14:22 ` Richard Palethorpe
0 siblings, 0 replies; 9+ messages in thread
From: Richard Palethorpe @ 2022-08-25 14:22 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-08-25 14:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-18 17:17 [LTP] [PATCH 1/1] bpf_prog0[5-7]: Run with kernel.unprivileged_bpf_disabled = 0 Petr Vorel
2022-08-18 17:50 ` Petr Vorel
2022-08-19 8:27 ` Cyril Hrubis
2022-08-19 10:28 ` Petr Vorel
2022-08-19 10:31 ` Petr Vorel
2022-08-19 10:43 ` Cyril Hrubis
2022-08-19 11:00 ` Cyril Hrubis
2022-08-19 12:08 ` Petr Vorel
2022-08-25 14:22 ` Richard Palethorpe
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).