linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Tissoires <bentiss@kernel.org>
To: Jiri Kosina <jikos@kernel.org>, Shuah Khan <shuah@kernel.org>,
	 Arnd Bergmann <arnd@kernel.org>
Cc: linux-input@vger.kernel.org, linux-kselftest@vger.kernel.org,
	 linux-kernel@vger.kernel.org,
	Benjamin Tissoires <bentiss@kernel.org>
Subject: [PATCH v3 2/3] selftests/hid: hidraw: forge wrong ioctls and tests them
Date: Fri, 12 Sep 2025 18:58:50 +0200	[thread overview]
Message-ID: <20250912-b4-hidraw-ioctls-v3-2-cd2c6efd8c20@kernel.org> (raw)
In-Reply-To: <20250912-b4-hidraw-ioctls-v3-0-cd2c6efd8c20@kernel.org>

We also need coverage for when the malicious user is not using the
proper ioctls definitions and tries to work around the driver.

Most of the scaffholding has been generated by claude-4-sonnet and then
carefully reviewed.

Suggested-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 tools/testing/selftests/hid/hidraw.c | 127 +++++++++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)

diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c
index 6d61d03e2ef05e1900fe5a3938d93421717b2621..d625772f8b7cf71fd94956d3a49d54ff44e2b34d 100644
--- a/tools/testing/selftests/hid/hidraw.c
+++ b/tools/testing/selftests/hid/hidraw.c
@@ -332,6 +332,133 @@ TEST_F(hidraw, ioctl_gfeature_invalid)
 	ASSERT_EQ(errno, EIO) TH_LOG("expected EIO, got errno %d", errno);
 }
 
+/*
+ * Test ioctl with incorrect nr bits
+ */
+TEST_F(hidraw, ioctl_invalid_nr)
+{
+	char buf[256] = {0};
+	int err;
+	unsigned int bad_cmd;
+
+	/*
+	 * craft an ioctl command with wrong _IOC_NR bits
+	 */
+	bad_cmd = _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x00, sizeof(buf)); /* 0 is not valid */
+
+	/* test the ioctl */
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("ioctl read-write with wrong _IOC_NR (0) should have failed");
+	ASSERT_EQ(errno, ENOTTY)
+		TH_LOG("expected ENOTTY for wrong read-write _IOC_NR (0), got errno %d", errno);
+
+	/*
+	 * craft an ioctl command with wrong _IOC_NR bits
+	 */
+	bad_cmd = _IOC(_IOC_READ, 'H', 0x00, sizeof(buf)); /* 0 is not valid */
+
+	/* test the ioctl */
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("ioctl read-only with wrong _IOC_NR (0) should have failed");
+	ASSERT_EQ(errno, ENOTTY)
+		TH_LOG("expected ENOTTY for wrong read-only _IOC_NR (0), got errno %d", errno);
+
+	/* also test with bigger number */
+	bad_cmd = _IOC(_IOC_READ, 'H', 0x42, sizeof(buf)); /* 0x42 is not valid as well */
+
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("ioctl read-only with wrong _IOC_NR (0x42) should have failed");
+	ASSERT_EQ(errno, ENOTTY)
+		TH_LOG("expected ENOTTY for wrong read-only _IOC_NR (0x42), got errno %d", errno);
+
+	/* also test with bigger number: 0x42 is not valid as well */
+	bad_cmd = _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x42, sizeof(buf));
+
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("ioctl read-write with wrong _IOC_NR (0x42) should have failed");
+	ASSERT_EQ(errno, ENOTTY)
+		TH_LOG("expected ENOTTY for wrong read-write _IOC_NR (0x42), got errno %d", errno);
+}
+
+/*
+ * Test ioctl with incorrect type bits
+ */
+TEST_F(hidraw, ioctl_invalid_type)
+{
+	char buf[256] = {0};
+	int err;
+	unsigned int bad_cmd;
+
+	/*
+	 * craft an ioctl command with wrong _IOC_TYPE bits
+	 */
+	bad_cmd = _IOC(_IOC_WRITE|_IOC_READ, 'I', 0x01, sizeof(buf)); /* 'I' should be 'H' */
+
+	/* test the ioctl */
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("ioctl with wrong _IOC_TYPE (I) should have failed");
+	ASSERT_EQ(errno, EINVAL) TH_LOG("expected EINVAL for wrong _IOC_NR, got errno %d", errno);
+}
+
+/*
+ * Test HIDIOCGFEATURE ioctl with incorrect _IOC_DIR bits
+ */
+TEST_F(hidraw, ioctl_gfeature_invalid_dir)
+{
+	__u8 buf[10] = {0};
+	int err;
+	unsigned int bad_cmd;
+
+	/* set report ID 1 in first byte */
+	buf[0] = 1;
+
+	/*
+	 * craft an ioctl command with wrong _IOC_DIR bits
+	 * HIDIOCGFEATURE should have _IOC_WRITE|_IOC_READ, let's use only _IOC_WRITE
+	 */
+	bad_cmd = _IOC(_IOC_WRITE, 'H', 0x07, sizeof(buf)); /* should be _IOC_WRITE|_IOC_READ */
+
+	/* try to get feature report with wrong direction bits */
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("HIDIOCGFEATURE with wrong _IOC_DIR should have failed");
+	ASSERT_EQ(errno, EINVAL) TH_LOG("expected EINVAL for wrong _IOC_DIR, got errno %d", errno);
+
+	/* also test with only _IOC_READ */
+	bad_cmd = _IOC(_IOC_READ, 'H', 0x07, sizeof(buf)); /* should be _IOC_WRITE|_IOC_READ */
+
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("HIDIOCGFEATURE with wrong _IOC_DIR should have failed");
+	ASSERT_EQ(errno, EINVAL) TH_LOG("expected EINVAL for wrong _IOC_DIR, got errno %d", errno);
+}
+
+/*
+ * Test read-only ioctl with incorrect _IOC_DIR bits
+ */
+TEST_F(hidraw, ioctl_readonly_invalid_dir)
+{
+	char buf[256] = {0};
+	int err;
+	unsigned int bad_cmd;
+
+	/*
+	 * craft an ioctl command with wrong _IOC_DIR bits
+	 * HIDIOCGRAWNAME should have _IOC_READ, let's use _IOC_WRITE
+	 */
+	bad_cmd = _IOC(_IOC_WRITE, 'H', 0x04, sizeof(buf)); /* should be _IOC_READ */
+
+	/* try to get device name with wrong direction bits */
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("HIDIOCGRAWNAME with wrong _IOC_DIR should have failed");
+	ASSERT_EQ(errno, EINVAL) TH_LOG("expected EINVAL for wrong _IOC_DIR, got errno %d", errno);
+
+	/* also test with _IOC_WRITE|_IOC_READ */
+	bad_cmd = _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x04, sizeof(buf)); /* should be only _IOC_READ */
+
+	err = ioctl(self->hidraw_fd, bad_cmd, buf);
+	ASSERT_LT(err, 0) TH_LOG("HIDIOCGRAWNAME with wrong _IOC_DIR should have failed");
+	ASSERT_EQ(errno, EINVAL) TH_LOG("expected EINVAL for wrong _IOC_DIR, got errno %d", errno);
+}
+
 /*
  * Test HIDIOCSFEATURE ioctl to set feature report
  */

-- 
2.51.0


  parent reply	other threads:[~2025-09-12 16:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-12 16:58 [PATCH v3 0/3] HID: hidraw: rework ioctls Benjamin Tissoires
2025-09-12 16:58 ` [PATCH v3 1/3] selftests/hid: hidraw: add more coverage for hidraw ioctls Benjamin Tissoires
2025-09-12 16:58 ` Benjamin Tissoires [this message]
2025-09-12 16:58 ` [PATCH v3 3/3] HID: hidraw: tighten ioctl command parsing Benjamin Tissoires
2025-09-17  9:38 ` [PATCH v3 0/3] HID: hidraw: rework ioctls Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250912-b4-hidraw-ioctls-v3-2-cd2c6efd8c20@kernel.org \
    --to=bentiss@kernel.org \
    --cc=arnd@kernel.org \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).