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

[Peter Senna Tschudin]

Hi,

Just a note to let you know that I am abandoning this patch. I will
modify existing tests instead of creating a a separate security test
like I attempted with this patch.

Thank you for your comments,

Peter

On 8/25/2025 4:09 PM, Michal Winiarski wrote:
> 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
>>