[...]
> >
> > @@ -44,6 +54,11 @@ void test_test_ima(void)
> > if (CHECK(!skel, "skel_load", "skeleton failed\n"))
> > goto close_prog;
> >
> > + ringbuf = ring_buffer__new(bpf_map__fd(skel->maps.ringbuf),
> > + process_sample, NULL, NULL);
> > + if (CHECK(!ringbuf, "ringbuf_create", "failed to create ringbuf\n"))
> > + goto close_prog;
>
> nit: could have used ASSERT_OK_PTR()
Updated this instance, I can separately clean some of the other places
to use the
ASSERT_* macros as well.
>
> > +
> > err = ima__attach(skel);
> > if (CHECK(err, "attach", "attach failed: %d\n", err))
> > goto close_prog;
> > @@ -60,11 +75,9 @@ void test_test_ima(void)
> > if (CHECK(err, "run_measured_process", "err = %d\n", err))
> > goto close_clean;
> >
> > - CHECK(skel->data->ima_hash_ret < 0, "ima_hash_ret",
> > - "ima_hash_ret = %ld\n", skel->data->ima_hash_ret);
> > -
> > - CHECK(skel->bss->ima_hash == 0, "ima_hash",
> > - "ima_hash = %lu\n", skel->bss->ima_hash);
> > + err = ring_buffer__poll(ringbuf, 1000);
>
> nit: given data should definitely be available here, could use
> ring_buffer__consume() instead and fail immediately if data is not
> there
Good idea. Done.
>
> > + ASSERT_EQ(err, 1, "num_samples_or_err");
> > + ASSERT_NEQ(ima_hash_from_bpf, 0, "ima_hash");
> >
> > close_clean:
> > snprintf(cmd, sizeof(cmd), "./ima_setup.sh cleanup %s", measured_dir);
> > diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c
> > index 86b21aff4bc5..dd0792204a21 100644
> > --- a/tools/testing/selftests/bpf/progs/ima.c
> > +++ b/tools/testing/selftests/bpf/progs/ima.c
> > @@ -9,20 +9,37 @@
> > #include <bpf/bpf_helpers.h>
> > #include <bpf/bpf_tracing.h>
> >
> > -long ima_hash_ret = -1;
> > -u64 ima_hash = 0;
> > u32 monitored_pid = 0;
> >
> > +struct {
> > + __uint(type, BPF_MAP_TYPE_RINGBUF);
> > + __uint(max_entries, 1 << 12);
> > +} ringbuf SEC(".maps");
[...]
> > + sample = bpf_ringbuf_reserve(&ringbuf, sizeof(u64), 0);
> > + if (!sample)
> > + return;
> >
> > - if (pid == monitored_pid)
> > - ima_hash_ret = bpf_ima_inode_hash(bprm->file->f_inode,
> > - &ima_hash, sizeof(ima_hash));
> > + *sample = ima_hash;
> > + bpf_ringbuf_submit(sample, BPF_RB_FORCE_WAKEUP);
>
> no need for BPF_RB_FORCE_WAKEUP, notification should happen
> deterministically. And further, if you use ring_buffer__consume() you
> won't even rely on notifications. Did you see any problems without
> this?
Yes, it works without BPF_RB_FORCE_WAKEUP too, I thought I had removed it,
which I clearly didn't :)
>
> > + }
> >
> > - return 0;
> > + return;
> > }
> > --
> > 2.30.0.365.g02bc693789-goog
> >
