[PATCH v4 i-g-t 0/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[PATCH v4 i-g-t 0/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Sobin Thomas]

This test validates that invalid string inputs are correctly rejected
by engine sysfs write. It ensures that the property values remain
unchanged when invalid inputs with large strings are provided.

Sobin Thomas (1):
  tests/intel/xe_sysfs_scheduler: Add invalid string test for engine
    sysfs properties

 tests/intel/xe_sysfs_scheduler.c | 98 ++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
-- 
2.34.1

[PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Sobin Thomas]

This test validates that invalid string inputs are correctly rejected
by engine sysfs write. It ensures that the property values remain
unchanged when invalid inputs are provided.

v4: Added subtests to test large (4k) random strings (kamil).
Replaced igt macro.

v3: Fixed the test logic. Fixed the tabs

v2: Added error check for return values for igt_sysfs_scanf and
igt_sysfs_printf. Removed the changes for fault injection in
tests/intel/xe_fault_injection.c

v1: Initial changes for checking error string.

Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
---
 tests/intel/xe_sysfs_scheduler.c | 98 ++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
index 4fc764f82..285689778 100644
--- a/tests/intel/xe_sysfs_scheduler.c
+++ b/tests/intel/xe_sysfs_scheduler.c
@@ -15,6 +15,16 @@
  *		unrepresentable intervals.
  * Test category: negative test
  *
+ * SUBTEST: %s-invalid-string
+ * Description: Test to check if %s arg[1] schedule parameter checks for
+ *		invalid string values.
+ * Test category: negative test
+ *
+ * SUBTEST: %s-invalid-large-string
+ * Description: Test to check if %s arg[1] schedule parameter checks for
+ *		large invalid strings (4k).
+ * Test category: negative test
+ *
  * SUBTEST: %s-min-max
  * Description: Test to check if %s arg[1] schedule parameter checks for
  *		min max values.
@@ -39,6 +49,46 @@
 #include "xe_drm.h"
 #include "xe/xe_query.h"
 
+#define STR_LENGTH 4096
+
+/*
+ * generate_random_string function is supposed to generate random
+ * string with more bias on alphabets than numeric this function
+ * provides random string based on the output of rand() and can
+ * sometime provide same class of characters if the output of
+ * rand() is same.
+ */
+static void generate_random_string(char *str, size_t length)
+{
+	int type = 0;
+	int digit_count = 0;
+	int max_digits = length / 4;
+
+	for (size_t i = 0; i < length; i++) {
+		if (digit_count >= max_digts)
+			type = rand() % 2;
+		else
+			type = rand() % 3;
+
+		switch (type) {
+		case 0:
+			str[i] = 'A' + (rand() % 26);
+			break;
+		case 1:
+			str[i] = 'a' + (rand() % 26);
+			break;
+		case 2:
+			str[i] = '0' + (rand() % 10);
+			digit_count++;
+			break;
+		default:
+			str[i] = '_';
+			break;
+		}
+	}
+	str[length] = '\0';
+}
+
 static void test_invalid(int xe, int engine, const char **property,
 			 uint16_t class, int gt)
 {
@@ -121,6 +171,52 @@ static void test_min_max(int xe, int engine, const char **property,
 	igt_assert_eq(set, default_max);
 }
 
+static void test_invalid_string(int xe, int engine, const char **property,
+				 uint16_t class, int gt)
+{
+	unsigned int saved, set;
+	static const char invalid_input[] = "999abc";
+
+	for (int i = 0; i < 3; i++) {
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &saved), 1);
+		igt_info("Initial %s: %u\n", property[i], saved);
+		/* Assert if the invalid write is returning negative error */
+		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
+			      invalid_input), 0);
+
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &set), 1);
+		/* Check if the values are unchanged. */
+		igt_assert_eq(set, saved);
+	}
+}
+
+static void test_invalid_large_string(int xe, int engine, const char **property,
+				       uint16_t class, int gt)
+{
+	unsigned int saved, set;
+	char *random_str;
+
+	random_str = (char *)calloc(STR_LENGTH, sizeof(char));
+	igt_assert_f(random_str, "Memory allocation failed\n");
+
+	generate_random_string(random_str, STR_LENGTH);
+	igt_debug("Generated random string: %.10s...\n", random_str);
+
+	for (int i = 0; i < 3; i++) {
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &saved), 1);
+		igt_info("Initial %s: %u\n", property[i], saved);
+
+		/* Assert if the invalid write is returning negative error */
+		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
+				random_str), 0);
+
+		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u", &set), 1);
+		/* Check if the values are unchanged. */
+		igt_assert_eq(set, saved);
+	}
+	free(random_str);
+}
+
 #define MAX_GTS 8
 igt_main
 {
@@ -129,6 +225,8 @@ igt_main
 		void (*fn)(int, int, const char **, uint16_t, int);
 	} tests[] = {
 		{ "invalid", test_invalid },
+		{ "invalid-string", test_invalid_string },
+		{ "invalid-large-string", test_invalid_large_string },
 		{ "min-max", test_min_max },
 		{ }
 	};
-- 
2.34.1

RE: [PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string test for engine sysfs properties

[Piatkowski, Dominik Karol]

Hi Sobin,

> -----Original Message-----
> From: Thomas, Sobin <sobin.thomas@intel.com>
> Sent: Tuesday, April 8, 2025 6:19 AM
> To: igt-dev@lists.freedesktop.org
> Cc: Piatkowski, Dominik Karol <dominik.karol.piatkowski@intel.com>;
> Konieczny, Kamil <kamil.konieczny@intel.com>
> Subject: [PATCH i-g-t 1/1] tests/intel/xe_sysfs_scheduler: Add invalid string
> test for engine sysfs properties

Please remember to add version number to patch subject.

> 
> This test validates that invalid string inputs are correctly rejected
> by engine sysfs write. It ensures that the property values remain
> unchanged when invalid inputs are provided.
> 
> v4: Added subtests to test large (4k) random strings (kamil).
> Replaced igt macro.
> 
> v3: Fixed the test logic. Fixed the tabs
> 
> v2: Added error check for return values for igt_sysfs_scanf and
> igt_sysfs_printf. Removed the changes for fault injection in
> tests/intel/xe_fault_injection.c
> 
> v1: Initial changes for checking error string.
> 
> Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
> ---
>  tests/intel/xe_sysfs_scheduler.c | 98
> ++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
> index 4fc764f82..285689778 100644
> --- a/tests/intel/xe_sysfs_scheduler.c
> +++ b/tests/intel/xe_sysfs_scheduler.c
> @@ -15,6 +15,16 @@
>   *		unrepresentable intervals.
>   * Test category: negative test
>   *
> + * SUBTEST: %s-invalid-string
> + * Description: Test to check if %s arg[1] schedule parameter checks for
> + *		invalid string values.
> + * Test category: negative test
> + *
> + * SUBTEST: %s-invalid-large-string
> + * Description: Test to check if %s arg[1] schedule parameter checks for
> + *		large invalid strings (4k).
> + * Test category: negative test
> + *
>   * SUBTEST: %s-min-max
>   * Description: Test to check if %s arg[1] schedule parameter checks for
>   *		min max values.
> @@ -39,6 +49,46 @@
>  #include "xe_drm.h"
>  #include "xe/xe_query.h"
> 
> +#define STR_LENGTH 4096
> +
> +/*
> + * generate_random_string function is supposed to generate random
> + * string with more bias on alphabets than numeric this function
> + * provides random string based on the output of rand() and can
> + * sometime provide same class of characters if the output of
> + * rand() is same.
> + */

Nitpick: this documentation could be improved. I propose:

/**
 * generate_random_string:
 * @str: pointer to string buffer
 * @length: length of string to generate, excluding null termination
 *
 * Generates random string that will always contain non-numerical characters.
 */

> +static void generate_random_string(char *str, size_t length)
> +{
> +	int type = 0;
> +	int digit_count = 0;
> +	int max_digits = length / 4;
> +
> +	for (size_t i = 0; i < length; i++) {
> +		if (digit_count >= max_digts)

digit_count > max_digits will never happen, but >= comparison will still work.

There is a typo as well, it should be `max_digits` instead of `max_digts`.
This causes build to fail.

> +			type = rand() % 2;
> +		else
> +			type = rand() % 3;
> +
> +		switch (type) {
> +		case 0:
> +			str[i] = 'A' + (rand() % 26);
> +			break;
> +		case 1:
> +			str[i] = 'a' + (rand() % 26);
> +			break;
> +		case 2:
> +			str[i] = '0' + (rand() % 10);
> +			digit_count++;
> +			break;
> +		default:
> +			str[i] = '_';
> +			break;
> +		}
> +	}
> +	str[length] = '\0';
> +}
> +
>  static void test_invalid(int xe, int engine, const char **property,
>  			 uint16_t class, int gt)
>  {
> @@ -121,6 +171,52 @@ static void test_min_max(int xe, int engine, const
> char **property,
>  	igt_assert_eq(set, default_max);
>  }
> 
> +static void test_invalid_string(int xe, int engine, const char **property,
> +				 uint16_t class, int gt)
> +{
> +	unsigned int saved, set;
> +	static const char invalid_input[] = "999abc";
> +
> +	for (int i = 0; i < 3; i++) {
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u",
> &saved), 1);
> +		igt_info("Initial %s: %u\n", property[i], saved);
> +		/* Assert if the invalid write is returning negative error */
> +		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
> +			      invalid_input), 0);
> +
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u",
> &set), 1);
> +		/* Check if the values are unchanged. */
> +		igt_assert_eq(set, saved);
> +	}
> +}
> +
> +static void test_invalid_large_string(int xe, int engine, const char **property,
> +				       uint16_t class, int gt)
> +{
> +	unsigned int saved, set;
> +	char *random_str;
> +
> +	random_str = (char *)calloc(STR_LENGTH, sizeof(char));

You can use malloc instead of calloc here, as we will fully overwrite the buffer
anyway.

Also, one very important note - you're allocating buffer of size STR_LENGTH,
but the generate_random_string function modifies random_str[STR_LENGTH] and
that is a buffer overflow. You need to allocate STR_LENGTH + 1 to accommodate
the null terminator. This needs to be fixed.

> +	igt_assert_f(random_str, "Memory allocation failed\n");
> +
> +	generate_random_string(random_str, STR_LENGTH);
> +	igt_debug("Generated random string: %.10s...\n", random_str);
> +
> +	for (int i = 0; i < 3; i++) {
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u",
> &saved), 1);
> +		igt_info("Initial %s: %u\n", property[i], saved);
> +
> +		/* Assert if the invalid write is returning negative error */
> +		igt_assert_lt(igt_sysfs_printf(engine, property[i], "%s",
> +				random_str), 0);

Please align this line.

With issues fixed,
Reviewed-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>

> +
> +		igt_assert_eq(igt_sysfs_scanf(engine, property[i], "%u",
> &set), 1);
> +		/* Check if the values are unchanged. */
> +		igt_assert_eq(set, saved);
> +	}
> +	free(random_str);
> +}
> +
>  #define MAX_GTS 8
>  igt_main
>  {
> @@ -129,6 +225,8 @@ igt_main
>  		void (*fn)(int, int, const char **, uint16_t, int);
>  	} tests[] = {
>  		{ "invalid", test_invalid },
> +		{ "invalid-string", test_invalid_string },
> +		{ "invalid-large-string", test_invalid_large_string },
>  		{ "min-max", test_min_max },
>  		{ }
>  	};
> --
> 2.34.1