[PATCH i-g-t] tests/intel/xe_sysfs_file_perm: Verify sysfs file permissions for security

[Michal Winiarski]

On Wed, Aug 13, 2025 at 03:19:47PM +0200, Peter Senna Tschudin wrote:
> This IGT adds checks for the 13 sysfs files identified in the Xe threat
> model, ensuring they are only writable by the root user. If any of these
> files are writable by non-root users, a warning is issued.

As sysfs is a proper uAPI, I'd expect we already have tests that
excercise those sysfs'es from functional angle.

> 
> When files are not present in the system, only informational messages
> are logged. No warnings or test aborts occur in such cases.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Michal Winiarski <michal.winiarski at intel.com>
> Signed-off-by: Peter Senna Tschudin <peter.senna at linux.intel.com>
> ---
>  tests/intel/xe_sysfs_file_perm.c | 578 +++++++++++++++++++++++++++++++
>  tests/meson.build                |   1 +
>  2 files changed, 579 insertions(+)
>  create mode 100644 tests/intel/xe_sysfs_file_perm.c
> 
> diff --git a/tests/intel/xe_sysfs_file_perm.c b/tests/intel/xe_sysfs_file_perm.c
> new file mode 100644
> index 000000000..2833de10c
> --- /dev/null
> +++ b/tests/intel/xe_sysfs_file_perm.c
> @@ -0,0 +1,578 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +

[...]

> +
> +igt_main
> +{
> +	struct igt_list_head fds_list;
> +	struct igt_list_head files_list;
> +	struct igt_list_head pattern_list;
> +
> +	/**
> +	 * Do not change the list below without checking the Xe threat
> +	 * modeling first. You can contact peter.senna at intel.com or
> +	 * michal.winiarski at intel.com for questions and support.
> +	 */
> +

And indeed - it looks like we do have at least one existing test for
each of those files.

> +	/* DO NOT CHANGE START */
> +	static const char * const gt_patterns[] = {
> +		"ccs_mode",

xe_compute

> +		"engines/[a-zA-Z0-9]+/job_timeout_ms",
> +		"engines/[a-zA-Z0-9]+/preempt_timeout_us",
> +		"engines/[a-zA-Z0-9]+/timeslice_duration_min",
> +		"engines/[a-zA-Z0-9]+/timeslice_duration_max",
> +		"engines/[a-zA-Z0-9]+/timeslice_duration_us",

xe_sysfs_scheduler

> +		"freq0/max_freq",
> +		"freq0/min_freq"

xe_gt_freq

> +	};
> +
> +	static const char * const hwmon_patterns[] = {
> +		"power[0-9]+_crit",
> +		"curr[0-9]+_crit",
> +		"power[0-9]+_max",
> +		"power[0-9]+_max_interval"

intel_hwmon

> +	};
> +
> +	static const char * const devices_patterns[] = {
> +		"vram_d3cold_threshold"

xe_pm

Access rights are just a property of some functionality, and I would
prefer to test it in the right context.
This could be handled as a subtest in a functional test (checking
whether the API being used has proper access rights), instead of having
a test that tries to check the access rights of everything that the
driver exposes.

Thanks,
-Michał

> +	};
> +
> +	struct dir_input dir_inputs[DIR_COUNT] = {
> +		{ GT_DIR, gt_patterns, ARRAY_SIZE(gt_patterns) },
> +		{ HWMON_DIR, hwmon_patterns, ARRAY_SIZE(hwmon_patterns) },
> +		{ DEVICES_DIR, devices_patterns, ARRAY_SIZE(devices_patterns) }
> +	};
> +	/* DO NOT CHANGE END */
> +
> +	igt_fixture {
> +		IGT_INIT_LIST_HEAD(&fds_list);
> +		IGT_INIT_LIST_HEAD(&pattern_list);
> +		IGT_INIT_LIST_HEAD(&files_list);
> +
> +		igt_require(prepare_patterns(dir_inputs, &pattern_list));
> +
> +		igt_require(fds_open_all(&fds_list));
> +		igt_require(search_files(&fds_list, &pattern_list, &files_list));
> +		igt_require(fds_close_and_free_list(&fds_list));
> +
> +		/* We do not warn or abort if not all patterns match
> +		 * because not all nodes are always available
> +		 */
> +		check_pattern_match(&pattern_list);
> +	}
> +
> +	igt_describe("Check if only root can write to important sysfs files.");
> +	igt_subtest("check-file-permissions")
> +		igt_assert(check_files_permissions(&files_list));
> +
> +	igt_fixture {
> +		struct pattern_node *pattern_node, *pattern_tmp;
> +		struct file_node *file_node, *file_tmp;
> +
> +		igt_list_for_each_entry_safe(pattern_node, pattern_tmp, &pattern_list, link) {
> +			/* pattern_node->pattern is static and const and
> +			 * should not be freed
> +			 */
> +			regfree(&pattern_node->regex);
> +			igt_list_del(&pattern_node->link);
> +			free(pattern_node);
> +		}
> +
> +		igt_list_for_each_entry_safe(file_node, file_tmp, &files_list, link) {
> +			free(file_node->full_path);
> +			igt_list_del(&file_node->link);
> +			free(file_node);
> +		}
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 5c01c64e9..d904dffcd 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -330,6 +330,7 @@ intel_xe_progs = [
>  	'xe_sriov_flr',
>  	'xe_sriov_scheduling',
>  	'xe_sysfs_defaults',
> +	'xe_sysfs_file_perm',
>  	'xe_sysfs_preempt_timeout',
>  	'xe_sysfs_scheduler',
>          'xe_sysfs_timeslice_duration',
> -- 
> 2.43.0
>