From nobody Fri Oct 3 19:11:46 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F24A232143C; Tue, 26 Aug 2025 12:39:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756211999; cv=none; b=Ek4i9vSRemWocm9UdO20E/bJ0WWNdx0o51OEDNbPsVXYg5JiXLb/umsrSKa+b0zjydZAyR1oqCwzo7I10KieBAmP6yIFRkKVfWi8Pc5nkur+2QP/n6T6o/zL138H2ooqV967blnpIh6DONe+egzsgLbxL6e8xzWGz4M9PQHQoM4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756211999; c=relaxed/simple; bh=uOAylKK8VYjOuoqIUOqP7NlLxP9wcPLBaaHlAl3e/N8=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=npM9kt3AMUpCJiYqs8nJRv7xfxT25CYMS98hOFHt/9ba4jpro5rWUNnPdD6+VGHeK/ZNLJ1JO/wRgFtJv9oGe0tcu/ND2j7vGHj7GFn9nilBymdpwlo5bPZ8S9g3Iru/okwsIp3944E3TE+k815b/YjV+aW5np9CDtVQFyF8A74= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KL9SCld0; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KL9SCld0" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9047CC19423; Tue, 26 Aug 2025 12:39:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1756211996; bh=uOAylKK8VYjOuoqIUOqP7NlLxP9wcPLBaaHlAl3e/N8=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=KL9SCld0w0YEcXlkPAAv3S+rvwsHjMXqw+E8QcTx+7aiZ+Zqzzg+MwuuVsrwVgiJt zAGl/+mWjRUeNVx9YISSRjJWIKx6iPF+pJrTi+GrDPJtsYzeOQV18piCnQhIbovNz2 P4K2xgdh/CAt0le+9vbhKVjTDwbMwXbQJWvcKIYTbr9fa4+bnCz4ABvZCM/3Sy8+ND PG6dGI53919QL935OxCri/3zEFqiDYM39JUmFWjYtJHQ8SS4+F+Ba/0yc8xNPB1qB5 6qXbwITSBBFIBpWvFOtqiXYsGjHv/+ST3NbgUj7w9oxskY3wHm3Vv+HZsEpmnQxL1N LGyQ2ywVODifg== From: Benjamin Tissoires Date: Tue, 26 Aug 2025 14:39:39 +0200 Subject: [PATCH v2 1/3] selftests/hid: hidraw: add more coverage for hidraw ioctls Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20250826-b4-hidraw-ioctls-v2-1-c7726b236719@kernel.org> References: <20250826-b4-hidraw-ioctls-v2-0-c7726b236719@kernel.org> In-Reply-To: <20250826-b4-hidraw-ioctls-v2-0-c7726b236719@kernel.org> To: Jiri Kosina , Shuah Khan , Arnd Bergmann Cc: linux-input@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Tissoires X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=ed25519-sha256; t=1756211993; l=11713; i=bentiss@kernel.org; s=20230215; h=from:subject:message-id; bh=uOAylKK8VYjOuoqIUOqP7NlLxP9wcPLBaaHlAl3e/N8=; b=g+S3obILI/Av7Zyapl2lmKuHNKUfnmkpHxZ9P8nyPbBMZ9Oqd1yiXWHWyv1AgMdvHU9uS/HDy 4qE3TKfDog6APJEGWPlzOfASWIii0tKQgEl0IqLvTb88roIBO4T1snX X-Developer-Key: i=bentiss@kernel.org; a=ed25519; pk=7D1DyAVh6ajCkuUTudt/chMuXWIJHlv2qCsRkIizvFw= Try to ensure all ioctls are having at least one test. Co-developed-by: claude-4-sonnet Signed-off-by: Benjamin Tissoires --- tools/testing/selftests/hid/hid_common.h | 6 + tools/testing/selftests/hid/hidraw.c | 346 +++++++++++++++++++++++++++= ++++ 2 files changed, 352 insertions(+) diff --git a/tools/testing/selftests/hid/hid_common.h b/tools/testing/selft= ests/hid/hid_common.h index f77f69c6657d0f0f66beb3b50bf4b126f6f63348..8085519c47cb505b901ac80f208= 7dc9a1aa2b9c0 100644 --- a/tools/testing/selftests/hid/hid_common.h +++ b/tools/testing/selftests/hid/hid_common.h @@ -230,6 +230,12 @@ static int uhid_event(struct __test_metadata *_metadat= a, int fd) break; case UHID_SET_REPORT: UHID_LOG("UHID_SET_REPORT from uhid-dev"); + + answer.type =3D UHID_SET_REPORT_REPLY; + answer.u.set_report_reply.id =3D ev.u.set_report.id; + answer.u.set_report_reply.err =3D 0; /* success */ + + uhid_write(_metadata, fd, &answer); break; default: TH_LOG("Invalid event from uhid-dev: %u", ev.type); diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests= /hid/hidraw.c index 821db37ba4bbef82e5cf4b44b6675666f87a12ad..6d61d03e2ef05e1900fe5a3938d= 93421717b2621 100644 --- a/tools/testing/selftests/hid/hidraw.c +++ b/tools/testing/selftests/hid/hidraw.c @@ -2,6 +2,9 @@ /* Copyright (c) 2022-2024 Red Hat */ =20 #include "hid_common.h" +#include +#include +#include =20 /* for older kernels */ #ifndef HIDIOCREVOKE @@ -215,6 +218,349 @@ TEST_F(hidraw, write_event_revoked) pthread_mutex_unlock(&uhid_output_mtx); } =20 +/* + * Test HIDIOCGRDESCSIZE ioctl to get report descriptor size + */ +TEST_F(hidraw, ioctl_rdescsize) +{ + int desc_size =3D 0; + int err; + + /* call HIDIOCGRDESCSIZE ioctl */ + err =3D ioctl(self->hidraw_fd, HIDIOCGRDESCSIZE, &desc_size); + ASSERT_EQ(err, 0) TH_LOG("HIDIOCGRDESCSIZE ioctl failed"); + + /* verify the size matches our test report descriptor */ + ASSERT_EQ(desc_size, sizeof(rdesc)) + TH_LOG("expected size %zu, got %d", sizeof(rdesc), desc_size); +} + +/* + * Test HIDIOCGRDESC ioctl to get report descriptor data + */ +TEST_F(hidraw, ioctl_rdesc) +{ + struct hidraw_report_descriptor desc; + int err; + + /* get the full report descriptor */ + desc.size =3D sizeof(rdesc); + err =3D ioctl(self->hidraw_fd, HIDIOCGRDESC, &desc); + ASSERT_EQ(err, 0) TH_LOG("HIDIOCGRDESC ioctl failed"); + + /* verify the descriptor data matches our test descriptor */ + ASSERT_EQ(memcmp(desc.value, rdesc, sizeof(rdesc)), 0) + TH_LOG("report descriptor data mismatch"); +} + +/* + * Test HIDIOCGRDESC ioctl with smaller buffer size + */ +TEST_F(hidraw, ioctl_rdesc_small_buffer) +{ + struct hidraw_report_descriptor desc; + int err; + size_t small_size =3D sizeof(rdesc) / 2; /* request half the descriptor s= ize */ + + /* get partial report descriptor */ + desc.size =3D small_size; + err =3D ioctl(self->hidraw_fd, HIDIOCGRDESC, &desc); + ASSERT_EQ(err, 0) TH_LOG("HIDIOCGRDESC ioctl failed with small buffer"); + + /* verify we got the first part of the descriptor */ + ASSERT_EQ(memcmp(desc.value, rdesc, small_size), 0) + TH_LOG("partial report descriptor data mismatch"); +} + +/* + * Test HIDIOCGRAWINFO ioctl to get device information + */ +TEST_F(hidraw, ioctl_rawinfo) +{ + struct hidraw_devinfo devinfo; + int err; + + /* get device info */ + err =3D ioctl(self->hidraw_fd, HIDIOCGRAWINFO, &devinfo); + ASSERT_EQ(err, 0) TH_LOG("HIDIOCGRAWINFO ioctl failed"); + + /* verify device info matches our test setup */ + ASSERT_EQ(devinfo.bustype, BUS_USB) + TH_LOG("expected bustype 0x03, got 0x%x", devinfo.bustype); + ASSERT_EQ(devinfo.vendor, 0x0001) + TH_LOG("expected vendor 0x0001, got 0x%x", devinfo.vendor); + ASSERT_EQ(devinfo.product, 0x0a37) + TH_LOG("expected product 0x0a37, got 0x%x", devinfo.product); +} + +/* + * Test HIDIOCGFEATURE ioctl to get feature report + */ +TEST_F(hidraw, ioctl_gfeature) +{ + __u8 buf[10] =3D {0}; + int err; + + /* set report ID 1 in first byte */ + buf[0] =3D 1; + + /* get feature report */ + err =3D ioctl(self->hidraw_fd, HIDIOCGFEATURE(sizeof(buf)), buf); + ASSERT_EQ(err, sizeof(feature_data)) TH_LOG("HIDIOCGFEATURE ioctl failed,= got %d", err); + + /* verify we got the expected feature data */ + ASSERT_EQ(buf[0], feature_data[0]) + TH_LOG("expected feature_data[0] =3D %d, got %d", feature_data[0], buf[0= ]); + ASSERT_EQ(buf[1], feature_data[1]) + TH_LOG("expected feature_data[1] =3D %d, got %d", feature_data[1], buf[1= ]); +} + +/* + * Test HIDIOCGFEATURE ioctl with invalid report ID + */ +TEST_F(hidraw, ioctl_gfeature_invalid) +{ + __u8 buf[10] =3D {0}; + int err; + + /* set invalid report ID (not 1) */ + buf[0] =3D 2; + + /* try to get feature report */ + err =3D ioctl(self->hidraw_fd, HIDIOCGFEATURE(sizeof(buf)), buf); + ASSERT_LT(err, 0) TH_LOG("HIDIOCGFEATURE should have failed with invalid = report ID"); + ASSERT_EQ(errno, EIO) TH_LOG("expected EIO, got errno %d", errno); +} + +/* + * Test HIDIOCSFEATURE ioctl to set feature report + */ +TEST_F(hidraw, ioctl_sfeature) +{ + __u8 buf[10] =3D {0}; + int err; + + /* prepare feature report data */ + buf[0] =3D 1; /* report ID */ + buf[1] =3D 0x42; + buf[2] =3D 0x24; + + /* set feature report */ + err =3D ioctl(self->hidraw_fd, HIDIOCSFEATURE(3), buf); + ASSERT_EQ(err, 3) TH_LOG("HIDIOCSFEATURE ioctl failed, got %d", err); + + /* + * Note: The uhid mock doesn't validate the set report data, + * so we just verify the ioctl succeeds + */ +} + +/* + * Test HIDIOCGINPUT ioctl to get input report + */ +TEST_F(hidraw, ioctl_ginput) +{ + __u8 buf[10] =3D {0}; + int err; + + /* set report ID 1 in first byte */ + buf[0] =3D 1; + + /* get input report */ + err =3D ioctl(self->hidraw_fd, HIDIOCGINPUT(sizeof(buf)), buf); + ASSERT_EQ(err, sizeof(feature_data)) TH_LOG("HIDIOCGINPUT ioctl failed, g= ot %d", err); + + /* verify we got the expected input data */ + ASSERT_EQ(buf[0], feature_data[0]) + TH_LOG("expected feature_data[0] =3D %d, got %d", feature_data[0], buf[0= ]); + ASSERT_EQ(buf[1], feature_data[1]) + TH_LOG("expected feature_data[1] =3D %d, got %d", feature_data[1], buf[1= ]); +} + +/* + * Test HIDIOCGINPUT ioctl with invalid report ID + */ +TEST_F(hidraw, ioctl_ginput_invalid) +{ + __u8 buf[10] =3D {0}; + int err; + + /* set invalid report ID (not 1) */ + buf[0] =3D 2; + + /* try to get input report */ + err =3D ioctl(self->hidraw_fd, HIDIOCGINPUT(sizeof(buf)), buf); + ASSERT_LT(err, 0) TH_LOG("HIDIOCGINPUT should have failed with invalid re= port ID"); + ASSERT_EQ(errno, EIO) TH_LOG("expected EIO, got errno %d", errno); +} + +/* + * Test HIDIOCSINPUT ioctl to set input report + */ +TEST_F(hidraw, ioctl_sinput) +{ + __u8 buf[10] =3D {0}; + int err; + + /* prepare input report data */ + buf[0] =3D 1; /* report ID */ + buf[1] =3D 0x55; + buf[2] =3D 0xAA; + + /* set input report */ + err =3D ioctl(self->hidraw_fd, HIDIOCSINPUT(3), buf); + ASSERT_EQ(err, 3) TH_LOG("HIDIOCSINPUT ioctl failed, got %d", err); + + /* + * Note: The uhid mock doesn't validate the set report data, + * so we just verify the ioctl succeeds + */ +} + +/* + * Test HIDIOCGOUTPUT ioctl to get output report + */ +TEST_F(hidraw, ioctl_goutput) +{ + __u8 buf[10] =3D {0}; + int err; + + /* set report ID 1 in first byte */ + buf[0] =3D 1; + + /* get output report */ + err =3D ioctl(self->hidraw_fd, HIDIOCGOUTPUT(sizeof(buf)), buf); + ASSERT_EQ(err, sizeof(feature_data)) TH_LOG("HIDIOCGOUTPUT ioctl failed, = got %d", err); + + /* verify we got the expected output data */ + ASSERT_EQ(buf[0], feature_data[0]) + TH_LOG("expected feature_data[0] =3D %d, got %d", feature_data[0], buf[0= ]); + ASSERT_EQ(buf[1], feature_data[1]) + TH_LOG("expected feature_data[1] =3D %d, got %d", feature_data[1], buf[1= ]); +} + +/* + * Test HIDIOCGOUTPUT ioctl with invalid report ID + */ +TEST_F(hidraw, ioctl_goutput_invalid) +{ + __u8 buf[10] =3D {0}; + int err; + + /* set invalid report ID (not 1) */ + buf[0] =3D 2; + + /* try to get output report */ + err =3D ioctl(self->hidraw_fd, HIDIOCGOUTPUT(sizeof(buf)), buf); + ASSERT_LT(err, 0) TH_LOG("HIDIOCGOUTPUT should have failed with invalid r= eport ID"); + ASSERT_EQ(errno, EIO) TH_LOG("expected EIO, got errno %d", errno); +} + +/* + * Test HIDIOCSOUTPUT ioctl to set output report + */ +TEST_F(hidraw, ioctl_soutput) +{ + __u8 buf[10] =3D {0}; + int err; + + /* prepare output report data */ + buf[0] =3D 1; /* report ID */ + buf[1] =3D 0x33; + buf[2] =3D 0xCC; + + /* set output report */ + err =3D ioctl(self->hidraw_fd, HIDIOCSOUTPUT(3), buf); + ASSERT_EQ(err, 3) TH_LOG("HIDIOCSOUTPUT ioctl failed, got %d", err); + + /* + * Note: The uhid mock doesn't validate the set report data, + * so we just verify the ioctl succeeds + */ +} + +/* + * Test HIDIOCGRAWNAME ioctl to get device name string + */ +TEST_F(hidraw, ioctl_rawname) +{ + char name[256] =3D {0}; + char expected_name[64]; + int err; + + /* get device name */ + err =3D ioctl(self->hidraw_fd, HIDIOCGRAWNAME(sizeof(name)), name); + ASSERT_GT(err, 0) TH_LOG("HIDIOCGRAWNAME ioctl failed, got %d", err); + + /* construct expected name based on device id */ + snprintf(expected_name, sizeof(expected_name), "test-uhid-device-%d", sel= f->hid.dev_id); + + /* verify the name matches expected pattern */ + ASSERT_EQ(strcmp(name, expected_name), 0) + TH_LOG("expected name '%s', got '%s'", expected_name, name); +} + +/* + * Test HIDIOCGRAWPHYS ioctl to get device physical address string + */ +TEST_F(hidraw, ioctl_rawphys) +{ + char phys[256] =3D {0}; + char expected_phys[64]; + int err; + + /* get device physical address */ + err =3D ioctl(self->hidraw_fd, HIDIOCGRAWPHYS(sizeof(phys)), phys); + ASSERT_GT(err, 0) TH_LOG("HIDIOCGRAWPHYS ioctl failed, got %d", err); + + /* construct expected phys based on device id */ + snprintf(expected_phys, sizeof(expected_phys), "%d", self->hid.dev_id); + + /* verify the phys matches expected value */ + ASSERT_EQ(strcmp(phys, expected_phys), 0) + TH_LOG("expected phys '%s', got '%s'", expected_phys, phys); +} + +/* + * Test HIDIOCGRAWUNIQ ioctl to get device unique identifier string + */ +TEST_F(hidraw, ioctl_rawuniq) +{ + char uniq[256] =3D {0}; + int err; + + /* get device unique identifier */ + err =3D ioctl(self->hidraw_fd, HIDIOCGRAWUNIQ(sizeof(uniq)), uniq); + ASSERT_GE(err, 0) TH_LOG("HIDIOCGRAWUNIQ ioctl failed, got %d", err); + + /* uniq is typically empty in our test setup */ + ASSERT_EQ(strlen(uniq), 0) TH_LOG("expected empty uniq, got '%s'", uniq); +} + +/* + * Test device string ioctls with small buffer sizes + */ +TEST_F(hidraw, ioctl_strings_small_buffer) +{ + char small_buf[8] =3D {0}; + char expected_name[64]; + int err; + + /* test HIDIOCGRAWNAME with small buffer */ + err =3D ioctl(self->hidraw_fd, HIDIOCGRAWNAME(sizeof(small_buf)), small_b= uf); + ASSERT_EQ(err, sizeof(small_buf)) + TH_LOG("HIDIOCGRAWNAME with small buffer failed, got %d", err); + + /* construct expected truncated name */ + snprintf(expected_name, sizeof(expected_name), "test-uhid-device-%d", sel= f->hid.dev_id); + + /* verify we got truncated name (first 8 chars, no null terminator guaran= teed) */ + ASSERT_EQ(strncmp(small_buf, expected_name, sizeof(small_buf)), 0) + TH_LOG("expected truncated name to match first %zu chars", sizeof(small_= buf)); + + /* Note: hidraw driver doesn't guarantee null termination when buffer is = too small */ +} + int main(int argc, char **argv) { return test_harness_run(argc, argv); --=20 2.51.0 From nobody Fri Oct 3 19:11:46 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F2562340D9E; Tue, 26 Aug 2025 12:39:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756211999; cv=none; b=a1/KNxnqTPiUqAgobFUVZeYtQhs/EjsR7Rr+FAEJXVCWchdcBoW6gfcRkAHfdsGQmIE5Tx0V3MGfOgSTF//NasqSlMQfIxXht0D45xlyaqgd2r/EJ0Ytf0hns9xFjnCl43oTmYAYKKU32hbDCYhoxeQd0uwwyysoOX85UayAvnU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756211999; c=relaxed/simple; bh=VCECJyDSKs2TTpt9lwvmhB5fy4SlZDLG640wB2jfk8I=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=FjP866/N+XQfMAsXNvZ3afgo5hl0cLU2XL6cICqb/HUk6NazbSx46633Hzy4Hl38h2hgf0DYKq2ZVZlPFLtLoTxGB9c/z54t4vY+9uEJ+V3DDBtnloxu7pmbowFcolUzE5oMK9DzWfaEPjM7wHzm1k/4q0bMIMqtVoCO3kH3SUA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=qcI6Twso; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="qcI6Twso" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3D3C1C113CF; Tue, 26 Aug 2025 12:39:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1756211998; bh=VCECJyDSKs2TTpt9lwvmhB5fy4SlZDLG640wB2jfk8I=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=qcI6Twso+0c+37Y9UNMH3/HsF3t6lFulri0NNMUYL4aotNU5Z3K8YGidGLvDSpieQ RQwc2gvY5onbQj+HYD4WcqJNNWsb7QVXkiJnT09BG+BbYwb3eBGFMhgOyAmBjE6Hv7 CopLq+Tf36a/jY3I5O5UrcpFrpLPHxGr5yE76Gi2CmHc32OvsQwg0H8FGVmeWbLbkI ARr2IZrbWXPzr5iPyzTWr2ImluPiOyblXzL7+sgoajuDY7xZqYyiK1EqBDO9IaYp+L BankLm4D8YlxHJ+0hwv412bHcYVBxL4IdFDKrkhw138yO3uyfSCI8HmclCK0vkncMT CC77lkOsQvXnA== From: Benjamin Tissoires Date: Tue, 26 Aug 2025 14:39:40 +0200 Subject: [PATCH v2 2/3] selftests/hid: hidraw: forge wrong ioctls and tests them Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20250826-b4-hidraw-ioctls-v2-2-c7726b236719@kernel.org> References: <20250826-b4-hidraw-ioctls-v2-0-c7726b236719@kernel.org> In-Reply-To: <20250826-b4-hidraw-ioctls-v2-0-c7726b236719@kernel.org> To: Jiri Kosina , Shuah Khan , Arnd Bergmann Cc: linux-input@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Tissoires X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=ed25519-sha256; t=1756211993; l=5453; i=bentiss@kernel.org; s=20230215; h=from:subject:message-id; bh=VCECJyDSKs2TTpt9lwvmhB5fy4SlZDLG640wB2jfk8I=; b=TJq8ncFg4FDeR+ln3/8/XvME1bGJbxZsCiSZre0/sw8Id6V896uwp1n5i9Odx674SiHHE9hJ8 FcdwR+87ZGYAOtS1of95Mld0mcr0GsILs+XsllCI18ekttp2hMGQrek X-Developer-Key: i=bentiss@kernel.org; a=ed25519; pk=7D1DyAVh6ajCkuUTudt/chMuXWIJHlv2qCsRkIizvFw= We also need coverage for when the malicious user is not using the proper ioctls definitions and tries to work around the driver. Suggested-by: Arnd Bergmann Co-developed-by: claude-4-sonnet Signed-off-by: Benjamin Tissoires --- 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..d625772f8b7cf71fd94956d3a49= d54ff44e2b34d 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); } =20 +/* + * Test ioctl with incorrect nr bits + */ +TEST_F(hidraw, ioctl_invalid_nr) +{ + char buf[256] =3D {0}; + int err; + unsigned int bad_cmd; + + /* + * craft an ioctl command with wrong _IOC_NR bits + */ + bad_cmd =3D _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x00, sizeof(buf)); /* 0 is n= ot valid */ + + /* test the ioctl */ + err =3D 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 =3D _IOC(_IOC_READ, 'H', 0x00, sizeof(buf)); /* 0 is not valid */ + + /* test the ioctl */ + err =3D ioctl(self->hidraw_fd, bad_cmd, buf); + ASSERT_LT(err, 0) TH_LOG("ioctl read-only with wrong _IOC_NR (0) should h= ave 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 =3D _IOC(_IOC_READ, 'H', 0x42, sizeof(buf)); /* 0x42 is not valid= as well */ + + err =3D ioctl(self->hidraw_fd, bad_cmd, buf); + ASSERT_LT(err, 0) TH_LOG("ioctl read-only with wrong _IOC_NR (0x42) shoul= d 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 =3D _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x42, sizeof(buf)); + + err =3D ioctl(self->hidraw_fd, bad_cmd, buf); + ASSERT_LT(err, 0) TH_LOG("ioctl read-write with wrong _IOC_NR (0x42) shou= ld 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] =3D {0}; + int err; + unsigned int bad_cmd; + + /* + * craft an ioctl command with wrong _IOC_TYPE bits + */ + bad_cmd =3D _IOC(_IOC_WRITE|_IOC_READ, 'I', 0x01, sizeof(buf)); /* 'I' sh= ould be 'H' */ + + /* test the ioctl */ + err =3D ioctl(self->hidraw_fd, bad_cmd, buf); + ASSERT_LT(err, 0) TH_LOG("ioctl with wrong _IOC_TYPE (I) should have fail= ed"); + ASSERT_EQ(errno, EINVAL) TH_LOG("expected EINVAL for wrong _IOC_NR, got e= rrno %d", errno); +} + +/* + * Test HIDIOCGFEATURE ioctl with incorrect _IOC_DIR bits + */ +TEST_F(hidraw, ioctl_gfeature_invalid_dir) +{ + __u8 buf[10] =3D {0}; + int err; + unsigned int bad_cmd; + + /* set report ID 1 in first byte */ + buf[0] =3D 1; + + /* + * craft an ioctl command with wrong _IOC_DIR bits + * HIDIOCGFEATURE should have _IOC_WRITE|_IOC_READ, let's use only _IOC_W= RITE + */ + bad_cmd =3D _IOC(_IOC_WRITE, 'H', 0x07, sizeof(buf)); /* should be _IOC_W= RITE|_IOC_READ */ + + /* try to get feature report with wrong direction bits */ + err =3D 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 =3D _IOC(_IOC_READ, 'H', 0x07, sizeof(buf)); /* should be _IOC_WR= ITE|_IOC_READ */ + + err =3D 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] =3D {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 =3D _IOC(_IOC_WRITE, 'H', 0x04, sizeof(buf)); /* should be _IOC_R= EAD */ + + /* try to get device name with wrong direction bits */ + err =3D 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 =3D _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x04, sizeof(buf)); /* should= be only _IOC_READ */ + + err =3D 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 */ --=20 2.51.0 From nobody Fri Oct 3 19:11:46 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D835E3431FE; Tue, 26 Aug 2025 12:40:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756212001; cv=none; b=XVAMs4/gpN7/B8hh3gjIsU04+5tcDZsjD7wkumYk8cAppFTCysZ922DJONkl1sJxGdGqAb/1ZC0k3cwtWe+tPEyYlOfBkXnctxc5LtzyQbtle15n0VM9ehQpjIqykxEdop1dOS9+iPhc96YSY/ym7zsgLjAye4TEQuszNTh+WOY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756212001; c=relaxed/simple; bh=LjmB04QPgjrYgaIwBfEs0zviXWneUfF/3NzneUzlG0E=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=BuhKvbifgosTHSD/DiibMhBG/sh72JbgRHZOwVPPGN3yWE7INP92a8tYItUBV409hSf5pBh4nDpi0+HEfU5ehNlw/pZ9EG3fBi7BpTFZqjgGmD1vsrBBzOvfUDiZmxmOYxho5O6PH+JQ2xhK7zSZuDx8f7+Oq5O/qOk9rC5YusI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ZvoJeHgY; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ZvoJeHgY" Received: by smtp.kernel.org (Postfix) with ESMTPSA id DEBC5C19424; Tue, 26 Aug 2025 12:39:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1756212000; bh=LjmB04QPgjrYgaIwBfEs0zviXWneUfF/3NzneUzlG0E=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=ZvoJeHgYirAothpZ1fo8dpGbuZ1gXoSqup1ue2GphXnmbcwU0BEB+w7JCcjPMlMZO oX2Jhpy9viJU9FDMbIMh2EVNRfJ8PyPB/m3r/n+kFrhKiK415y5QW3YHxSEtclIUDg LaTDgC0U1kW/MzKFNvuqrZeuwPNcuoGeolVKUkd4/vpPy+grcSQh3dO5UFbHt69Nf/ r6OKViMBTlttUipH7DvZyGy5mPjUl3Bdf/cckaCAeExA0bPr/c/zV8T/U4bNYQ6/9S Lkn3Rsyvxvf8wK3oTq05MOu3TqQJcvjkW3pf0dwSWdW8KIKOdzpknl01F/kFg/NERq NVeW1exRxPCQQ== From: bentiss@kernel.org Date: Tue, 26 Aug 2025 14:39:41 +0200 Subject: [PATCH v2 3/3] HID: tighten ioctl command parsing Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20250826-b4-hidraw-ioctls-v2-3-c7726b236719@kernel.org> References: <20250826-b4-hidraw-ioctls-v2-0-c7726b236719@kernel.org> In-Reply-To: <20250826-b4-hidraw-ioctls-v2-0-c7726b236719@kernel.org> To: Jiri Kosina , Shuah Khan , Arnd Bergmann Cc: linux-input@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Tissoires , Arnd Bergmann X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=ed25519-sha256; t=1756211993; l=9012; i=bentiss@kernel.org; s=20230215; h=from:subject:message-id; bh=KZHaOS6e7iHYeZyNxItBcY9Qs+NZLFJ6hQpelRnM5kk=; b=i380UFR0uXVHXY2lFZNgDq58bgE+nWEzzbVgLo679JKVUyFr402smHWrAgBHwbtLG8yYbE/4e PTec60LEg/hDufvUUgcXfJYNvN7fl61CAlqM8jYjvt/uIwWzznmUkV9 X-Developer-Key: i=bentiss@kernel.org; a=ed25519; pk=7D1DyAVh6ajCkuUTudt/chMuXWIJHlv2qCsRkIizvFw= From: Arnd Bergmann The handling for variable-length ioctl commands in hidraw_ioctl() is rather complex and the check for the data direction is incomplete. Simplify this code by factoring out the various ioctls grouped by dir and size, and using a switch() statement with the size masked out, to ensure the rest of the command is correctly matched. Fixes: 9188e79ec3fd ("HID: add phys and name ioctls to hidraw") Signed-off-by: Arnd Bergmann Signed-off-by: Benjamin Tissoires --- drivers/hid/hidraw.c | 224 ++++++++++++++++++++++++----------------= ---- include/uapi/linux/hidraw.h | 2 + 2 files changed, 124 insertions(+), 102 deletions(-) diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c index c887f48756f4be2a4bac03128f2885bde96c1e39..bbd6f23bce78951c7d667ff5c1c= 923cee3509e3f 100644 --- a/drivers/hid/hidraw.c +++ b/drivers/hid/hidraw.c @@ -394,27 +394,15 @@ static int hidraw_revoke(struct hidraw_list *list) return 0; } =20 -static long hidraw_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) +static long hidraw_fixed_size_ioctl(struct file *file, struct hidraw *dev,= unsigned int cmd, + void __user *arg) { - struct inode *inode =3D file_inode(file); - unsigned int minor =3D iminor(inode); - long ret =3D 0; - struct hidraw *dev; - struct hidraw_list *list =3D file->private_data; - void __user *user_arg =3D (void __user*) arg; - - down_read(&minors_rwsem); - dev =3D hidraw_table[minor]; - if (!dev || !dev->exist || hidraw_is_revoked(list)) { - ret =3D -ENODEV; - goto out; - } + struct hid_device *hid =3D dev->hid; =20 switch (cmd) { case HIDIOCGRDESCSIZE: - if (put_user(dev->hid->rsize, (int __user *)arg)) - ret =3D -EFAULT; + if (put_user(hid->rsize, (int __user *)arg)) + return -EFAULT; break; =20 case HIDIOCGRDESC: @@ -422,113 +410,145 @@ static long hidraw_ioctl(struct file *file, unsigne= d int cmd, __u32 len; =20 if (get_user(len, (int __user *)arg)) - ret =3D -EFAULT; - else if (len > HID_MAX_DESCRIPTOR_SIZE - 1) - ret =3D -EINVAL; - else if (copy_to_user(user_arg + offsetof( - struct hidraw_report_descriptor, - value[0]), - dev->hid->rdesc, - min(dev->hid->rsize, len))) - ret =3D -EFAULT; + return -EFAULT; + + if (len > HID_MAX_DESCRIPTOR_SIZE - 1) + return -EINVAL; + + if (copy_to_user(arg + offsetof( + struct hidraw_report_descriptor, + value[0]), + hid->rdesc, + min(hid->rsize, len))) + return -EFAULT; + break; } case HIDIOCGRAWINFO: { struct hidraw_devinfo dinfo; =20 - dinfo.bustype =3D dev->hid->bus; - dinfo.vendor =3D dev->hid->vendor; - dinfo.product =3D dev->hid->product; - if (copy_to_user(user_arg, &dinfo, sizeof(dinfo))) - ret =3D -EFAULT; + dinfo.bustype =3D hid->bus; + dinfo.vendor =3D hid->vendor; + dinfo.product =3D hid->product; + if (copy_to_user(arg, &dinfo, sizeof(dinfo))) + return -EFAULT; break; } case HIDIOCREVOKE: { - if (user_arg) - ret =3D -EINVAL; - else - ret =3D hidraw_revoke(list); - break; + struct hidraw_list *list =3D file->private_data; + + if (arg) + return -EINVAL; + + return hidraw_revoke(list); } default: - { - struct hid_device *hid =3D dev->hid; - if (_IOC_TYPE(cmd) !=3D 'H') { - ret =3D -EINVAL; - break; - } + /* + * None of the above ioctls can return -EAGAIN, so + * use it as a marker that we need to check variable + * length ioctls. + */ + return -EAGAIN; + } =20 - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCSFEATURE(0))) { - int len =3D _IOC_SIZE(cmd); - ret =3D hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT); - break; - } - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCGFEATURE(0))) { - int len =3D _IOC_SIZE(cmd); - ret =3D hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT); - break; - } + return 0; +} =20 - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCSINPUT(0))) { - int len =3D _IOC_SIZE(cmd); - ret =3D hidraw_send_report(file, user_arg, len, HID_INPUT_REPORT); - break; - } - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCGINPUT(0))) { - int len =3D _IOC_SIZE(cmd); - ret =3D hidraw_get_report(file, user_arg, len, HID_INPUT_REPORT); - break; - } +static long hidraw_rw_variable_size_ioctl(struct file *file, struct hidraw= *dev, unsigned int cmd, + void __user *user_arg) +{ + int len =3D _IOC_SIZE(cmd); + + switch (cmd & ~IOCSIZE_MASK) { + case HIDIOCSFEATURE(0): + return hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT); + case HIDIOCGFEATURE(0): + return hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT); + case HIDIOCSINPUT(0): + return hidraw_send_report(file, user_arg, len, HID_INPUT_REPORT); + case HIDIOCGINPUT(0): + return hidraw_get_report(file, user_arg, len, HID_INPUT_REPORT); + case HIDIOCSOUTPUT(0): + return hidraw_send_report(file, user_arg, len, HID_OUTPUT_REPORT); + case HIDIOCGOUTPUT(0): + return hidraw_get_report(file, user_arg, len, HID_OUTPUT_REPORT); + } =20 - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCSOUTPUT(0))) { - int len =3D _IOC_SIZE(cmd); - ret =3D hidraw_send_report(file, user_arg, len, HID_OUTPUT_REPORT); - break; - } - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCGOUTPUT(0))) { - int len =3D _IOC_SIZE(cmd); - ret =3D hidraw_get_report(file, user_arg, len, HID_OUTPUT_REPORT); - break; - } + return -EINVAL; +} =20 - /* Begin Read-only ioctls. */ - if (_IOC_DIR(cmd) !=3D _IOC_READ) { - ret =3D -EINVAL; - break; - } +static long hidraw_ro_variable_size_ioctl(struct file *file, struct hidraw= *dev, unsigned int cmd, + void __user *user_arg) +{ + struct hid_device *hid =3D dev->hid; + int len =3D _IOC_SIZE(cmd); + int field_len; + + switch (cmd & ~IOCSIZE_MASK) { + case HIDIOCGRAWNAME(0): + field_len =3D strlen(hid->name) + 1; + if (len > field_len) + len =3D field_len; + return copy_to_user(user_arg, hid->name, len) ? -EFAULT : len; + case HIDIOCGRAWPHYS(0): + field_len =3D strlen(hid->phys) + 1; + if (len > field_len) + len =3D field_len; + return copy_to_user(user_arg, hid->phys, len) ? -EFAULT : len; + case HIDIOCGRAWUNIQ(0): + field_len =3D strlen(hid->uniq) + 1; + if (len > field_len) + len =3D field_len; + return copy_to_user(user_arg, hid->uniq, len) ? -EFAULT : len; + } =20 - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCGRAWNAME(0))) { - int len =3D strlen(hid->name) + 1; - if (len > _IOC_SIZE(cmd)) - len =3D _IOC_SIZE(cmd); - ret =3D copy_to_user(user_arg, hid->name, len) ? - -EFAULT : len; - break; - } + return -EINVAL; +} =20 - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCGRAWPHYS(0))) { - int len =3D strlen(hid->phys) + 1; - if (len > _IOC_SIZE(cmd)) - len =3D _IOC_SIZE(cmd); - ret =3D copy_to_user(user_arg, hid->phys, len) ? - -EFAULT : len; - break; - } +static long hidraw_ioctl(struct file *file, unsigned int cmd, unsigned lon= g arg) +{ + struct inode *inode =3D file_inode(file); + unsigned int minor =3D iminor(inode); + struct hidraw *dev; + struct hidraw_list *list =3D file->private_data; + void __user *user_arg =3D (void __user *)arg; + int ret; =20 - if (_IOC_NR(cmd) =3D=3D _IOC_NR(HIDIOCGRAWUNIQ(0))) { - int len =3D strlen(hid->uniq) + 1; - if (len > _IOC_SIZE(cmd)) - len =3D _IOC_SIZE(cmd); - ret =3D copy_to_user(user_arg, hid->uniq, len) ? - -EFAULT : len; - break; - } - } + down_read(&minors_rwsem); + dev =3D hidraw_table[minor]; + if (!dev || !dev->exist || hidraw_is_revoked(list)) { + ret =3D -ENODEV; + goto out; + } + + if (_IOC_TYPE(cmd) !=3D 'H') { + ret =3D -EINVAL; + goto out; + } =20 + if (_IOC_NR(cmd) > HIDIOCTL_LAST || _IOC_NR(cmd) =3D=3D 0) { ret =3D -ENOTTY; + goto out; } + + ret =3D hidraw_fixed_size_ioctl(file, dev, cmd, user_arg); + if (ret !=3D -EAGAIN) + goto out; + + switch (_IOC_DIR(cmd)) { + case (_IOC_READ | _IOC_WRITE): + ret =3D hidraw_rw_variable_size_ioctl(file, dev, cmd, user_arg); + break; + case _IOC_READ: + ret =3D hidraw_ro_variable_size_ioctl(file, dev, cmd, user_arg); + break; + default: + /* Any other IOC_DIR is wrong */ + ret =3D -EINVAL; + } + out: up_read(&minors_rwsem); return ret; diff --git a/include/uapi/linux/hidraw.h b/include/uapi/linux/hidraw.h index d5ee269864e07fcaba481fa285bacbd98739e44f..ebd701b3c18d9d7465880199091= 933f13f2e1128 100644 --- a/include/uapi/linux/hidraw.h +++ b/include/uapi/linux/hidraw.h @@ -48,6 +48,8 @@ struct hidraw_devinfo { #define HIDIOCGOUTPUT(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0C, len) #define HIDIOCREVOKE _IOW('H', 0x0D, int) /* Revoke device access */ =20 +#define HIDIOCTL_LAST _IOC_NR(HIDIOCREVOKE) + #define HIDRAW_FIRST_MINOR 0 #define HIDRAW_MAX_DEVICES 64 /* number of reports to buffer */ --=20 2.51.0