Thanks for picking this up and adding the tests, much appreciated
On Tue, Aug 27, 2024 at 12:47:53AM +0900, Benjamin Tissoires wrote:
> Add 4 tests for the new revoke ioctl, for read/write/ioctl and poll.
>
> Signed-off-by: Benjamin Tissoires <bentiss@xxxxxxxxxx>
>
> ---
>
> new in v4
> ---
> tools/testing/selftests/hid/hidraw.c | 143 +++++++++++++++++++++++++++++++++++
> 1 file changed, 143 insertions(+)
>
> diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c
> index f8c933476dcd..669eada8886b 100644
> --- a/tools/testing/selftests/hid/hidraw.c
> +++ b/tools/testing/selftests/hid/hidraw.c
> @@ -19,6 +19,11 @@
> __typeof__(b) _b = (b); \
> _a < _b ? _a : _b; })
>
> +/* for older kernels */
> +#ifndef HIDIOCREVOKE
> +#define HIDIOCREVOKE _IOW('H', 0x0D, int) /* Revoke device access */
> +#endif /* HIDIOCREVOKE */
> +
> static unsigned char rdesc[] = {
> 0x06, 0x00, 0xff, /* Usage Page (Vendor Defined Page 1) */
> 0x09, 0x21, /* Usage (Vendor Usage 0x21) */
> @@ -516,6 +521,144 @@ TEST_F(hidraw, raw_event)
> ASSERT_EQ(buf[1], 42);
> }
>
> +/*
> + * After initial opening/checks of hidraw, revoke the hidraw
> + * node and check that we can not read any more data.
> + */
> +TEST_F(hidraw, raw_event_revoked)
> +{
> + __u8 buf[10] = {0};
> + int err;
> +
> + /* inject one event */
> + buf[0] = 1;
> + buf[1] = 42;
> + uhid_send_event(_metadata, self->uhid_fd, buf, 6);
> +
> + /* read the data from hidraw */
> + memset(buf, 0, sizeof(buf));
> + err = read(self->hidraw_fd, buf, sizeof(buf));
> + ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
> + ASSERT_EQ(buf[0], 1);
> + ASSERT_EQ(buf[1], 42);
> +
> + /* call the revoke ioctl */
> + err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
> + ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
> +
> + /* inject one other event */
> + buf[0] = 1;
> + buf[1] = 43;
> + uhid_send_event(_metadata, self->uhid_fd, buf, 6);
> +
> + /* read the data from hidraw */
> + memset(buf, 0, sizeof(buf));
> + err = read(self->hidraw_fd, buf, sizeof(buf));
> + ASSERT_EQ(err, -1) TH_LOG("read_hidraw");
do you want to check for errno == ENODEV here to avoid false positives?
Shouldn't really happen in this test suite but it costs very little.
Same for the various cases below.
With or without - series:
Reviewed-by: Peter Hutterer <peter.hutterer@xxxxxxxxx>
Cheers,
Peter
