On Thu, Mar 12, 2020 at 1:36 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> On Thu, Mar 12, 2020 at 4:40 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> > Currently the code sets it to at most 128K, but this is not enough in
> > some aarch64/ppc64le environments. Therefore, stop guessing the magic
> > threshold and just set it straight to RLIM_INFINITY.
> >
> > Fixes: 8f0f980a4ad5 ("selinux-testsuite: Add BPF tests")
> > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > ---
> > tests/bpf/bpf_common.c | 16 ++++------------
> > 1 file changed, 4 insertions(+), 12 deletions(-)
>
> I have to make a similar fix to the audit-testsuite earlier this week,
> I didn't think to check the selinux-testsuite because the bpf tests
> were running just fine on my aarch64 test system. Sorry about that, I
> should have checked regardless.
No problem, I had kept the bpf subtest disabled on non-x86_64 in our
testing for some time, since it was failing there and I didn't know
why. Once I saw your audit-testsuite patch I realized that it was
probably the same issue so I did some testing and indeed the
RLIMIT_MEMLOCK was the cause so I put together a patch.
>
> One small style nit below, but otherwise ...
>
> Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx>
>
> > diff --git a/tests/bpf/bpf_common.c b/tests/bpf/bpf_common.c
> > index 681e2eb..f499034 100644
> > --- a/tests/bpf/bpf_common.c
> > +++ b/tests/bpf/bpf_common.c
> > @@ -41,24 +41,16 @@ int create_bpf_prog(void)
> >
> > /*
> > * The default RLIMIT_MEMLOCK is normally 64K, however BPF map/prog requires
> > - * more than this so double it unless RLIM_INFINITY is set.
> > + * more than this (the actual threshold varying across arches) so set it to
> > + * RLIM_INFINITY.
> > */
> > void bpf_setrlimit(void)
> > {
> > int result;
> > struct rlimit r;
> >
> > - result = getrlimit(RLIMIT_MEMLOCK, &r);
> > - if (result < 0) {
> > - fprintf(stderr, "Failed to get resource limit: %s\n",
> > - strerror(errno));
> > - exit(-1);
> > - }
> > -
> > - if (r.rlim_cur != RLIM_INFINITY && r.rlim_cur <= (64 * 1024)) {
> > - r.rlim_cur *= 2;
> > - r.rlim_max *= 2;
> > - }
> > + r.rlim_cur = RLIM_INFINITY;
> > + r.rlim_max = RLIM_INFINITY;
>
> If you really want to simplify things you could assign those values
> when you declare "r":
>
> struct rlimit r = {
> .rlim_cur = RLIM_INFINITY,
> .rlim_max = RLIM_INFINITY };
True, but I don't think the difference is big enough to respin the patch.
>
> > result = setrlimit(RLIMIT_MEMLOCK, &r);
> > if (result < 0) {
> > --
> > 2.24.1
>
> --
> paul moore
> www.paul-moore.com
>
--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
