[PATCH v2 11/14] fs/resctrl: Use stricter checks on input to cpus/cpus_list file

Reinette Chatre posted 14 patches 2 weeks ago
[PATCH v2 11/14] fs/resctrl: Use stricter checks on input to cpus/cpus_list file
Posted by Reinette Chatre 2 weeks ago
Callbacks handling writes to resctrl files are called via
kernfs_fop_write_iter() that ensures that the resctrl callback is never
called with a NULL buffer. Even if user space issues a write() with a NULL
buffer and zero count the resctrl callback receives a valid buffer
terminated with '\0' and the number of bytes parameter set to zero.

The NULL buffer check at the beginning of rdtgroup_cpus_write(), while
making no assumptions about caller behavior, is not useful. An empty
buffer is transformed into an empty cpumask that is passed through entire
flow that results in no changes.

Ensure that the user provided buffer contains some data before attempting
to parse it using the same check as other resctrl files (the familiar
"nbytes == 0"). The custom is for resctrl file callbacks to fail on an
empty buffer and this brings interactions with the cpus/cpus_list file in
line with this custom. The risk is that if there exists a user space that
uses empty writes to this specific file then those successful interactions
will start failing.

Exit right away if there was no failure yet no cpumask could be created
from the input. It is of no use to pass an empty cpumask through the
entire flow, just return with success to short-circuit the existing
behavior.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 fs/resctrl/rdtgroup.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 6fa5c8f14e3a..5f3d89c7757c 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -515,7 +515,7 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 	struct rdtgroup *rdtgrp;
 	int ret;
 
-	if (!buf)
+	if (!buf || nbytes == 0)
 		return -EINVAL;
 
 	if (!zalloc_cpumask_var(&tmpmask, GFP_KERNEL))
@@ -555,6 +555,9 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
 		goto unlock;
 	}
 
+	if (cpumask_empty(newmask))
+		goto unlock;
+
 	/* check that user didn't specify any offline cpus */
 	cpumask_andnot(tmpmask, newmask, cpu_online_mask);
 	if (!cpumask_empty(tmpmask)) {
-- 
2.50.1
Re: [PATCH v2 11/14] fs/resctrl: Use stricter checks on input to cpus/cpus_list file
Posted by Reinette Chatre 1 week, 2 days ago

On 3/20/26 3:03 PM, Reinette Chatre wrote:
> Callbacks handling writes to resctrl files are called via
> kernfs_fop_write_iter() that ensures that the resctrl callback is never
> called with a NULL buffer. Even if user space issues a write() with a NULL
> buffer and zero count the resctrl callback receives a valid buffer
> terminated with '\0' and the number of bytes parameter set to zero.
> 
> The NULL buffer check at the beginning of rdtgroup_cpus_write(), while
> making no assumptions about caller behavior, is not useful. An empty
> buffer is transformed into an empty cpumask that is passed through entire
> flow that results in no changes.
> 
> Ensure that the user provided buffer contains some data before attempting
> to parse it using the same check as other resctrl files (the familiar
> "nbytes == 0"). The custom is for resctrl file callbacks to fail on an
> empty buffer and this brings interactions with the cpus/cpus_list file in
> line with this custom. The risk is that if there exists a user space that
> uses empty writes to this specific file then those successful interactions
> will start failing.
> 
> Exit right away if there was no failure yet no cpumask could be created
> from the input. It is of no use to pass an empty cpumask through the
> entire flow, just return with success to short-circuit the existing
> behavior.
> 
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> ---

Chris Samuel reported that this breaks a legitimate use case where user space
could use an empty write, for example write(fd, NULL, 0), to remove all CPUs
from a resource group.

This change thus ended up being too strict and should be dropped.

Reinette