Re: [PATCH v4 3/3] selftests/hid: Add HIDIOCREVOKE tests

[Peter Hutterer <peter.hutterer@who-t.net>]

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@kernel.org>
> 
> ---
> 
> 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@who-t.net>

Cheers,
  Peter