[PATCH v2] genirq: procfs: Make smp_affinity read-only for interrupts that userspace can't set

[PATCH v2] genirq: procfs: Make smp_affinity read-only for interrupts that userspace can't set

[Jeff Xie]

The kernel already knows at the time of interrupt allocation that the
affinity cannot be controlled by userspace and therefore creating the
file with write permissions is wrong.

Therefore set the file permissions to read-only for such interrupts.

Signed-off-by: Jeff Xie <jeff.xie@linux.dev>
---
v2:
- Updated the description suggested by tglx
- Corrected the return value from -EIO to -EPERM when the userspace can't set the affinity

 kernel/irq/proc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 8cccdf40725a..7b3a4c92d148 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -142,7 +142,7 @@ static ssize_t write_irq_affinity(int type, struct file *file,
 	int err;
 
 	if (!irq_can_set_affinity_usr(irq) || no_irq_affinity)
-		return -EIO;
+		return -EPERM;
 
 	if (!zalloc_cpumask_var(&new_value, GFP_KERNEL))
 		return -ENOMEM;
@@ -340,6 +340,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 	static DEFINE_MUTEX(register_lock);
 	void __maybe_unused *irqp = (void *)(unsigned long) irq;
 	char name [MAX_NAMELEN];
+	umode_t umode = S_IRUGO;
 
 	if (!root_irq_dir || (desc->irq_data.chip == &no_irq_chip))
 		return;
@@ -362,8 +363,11 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 		goto out_unlock;
 
 #ifdef CONFIG_SMP
+	if (irq_can_set_affinity_usr(desc->irq_data.irq))
+		umode |= S_IWUSR;
+
 	/* create /proc/irq/<irq>/smp_affinity */
-	proc_create_data("smp_affinity", 0644, desc->dir,
+	proc_create_data("smp_affinity", umode, desc->dir,
 			 &irq_affinity_proc_ops, irqp);
 
 	/* create /proc/irq/<irq>/affinity_hint */
@@ -371,7 +375,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 			irq_affinity_hint_proc_show, irqp);
 
 	/* create /proc/irq/<irq>/smp_affinity_list */
-	proc_create_data("smp_affinity_list", 0644, desc->dir,
+	proc_create_data("smp_affinity_list", umode, desc->dir,
 			 &irq_affinity_list_proc_ops, irqp);
 
 	proc_create_single_data("node", 0444, desc->dir, irq_node_proc_show,
-- 
2.34.1

Re: [PATCH v2] genirq: procfs: Make smp_affinity read-only for interrupts that userspace can't set

[Thomas Gleixner]

On Sun, Aug 25 2024 at 21:19, Jeff Xie wrote:
> The kernel already knows at the time of interrupt allocation that the
> affinity cannot be controlled by userspace and therefore creating the
> file with write permissions is wrong.
>
> Therefore set the file permissions to read-only for such interrupts.
>
> Signed-off-by: Jeff Xie <jeff.xie@linux.dev>
> ---
> v2:
> - Updated the description suggested by tglx
> - Corrected the return value from -EIO to -EPERM when the userspace can't set the affinity
>
>  kernel/irq/proc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
> index 8cccdf40725a..7b3a4c92d148 100644
> --- a/kernel/irq/proc.c
> +++ b/kernel/irq/proc.c
> @@ -142,7 +142,7 @@ static ssize_t write_irq_affinity(int type, struct file *file,
>  	int err;
>  
>  	if (!irq_can_set_affinity_usr(irq) || no_irq_affinity)
> -		return -EIO;
> +		return -EPERM;

I drop this hunk as it is unrelated to $subject. That want's to be a
separate patch. Documentation/process clearly states:

         Solve only one problem per patch.

Thanks,

        tglx

Re: [PATCH v2] genirq: procfs: Make smp_affinity read-only for interrupts that userspace can't set

[jeff.xie]

August 26, 2024 at 6:55 PM, "Thomas Gleixner" <tglx@linutronix.de> wrote:



> 
> On Sun, Aug 25 2024 at 21:19, Jeff Xie wrote:
> 
> > 
> > The kernel already knows at the time of interrupt allocation that the
> > 
> >  affinity cannot be controlled by userspace and therefore creating the
> > 
> >  file with write permissions is wrong.
> > 
> >  Therefore set the file permissions to read-only for such interrupts.
> > 
> >  Signed-off-by: Jeff Xie <jeff.xie@linux.dev>
> > 
> >  ---
> > 
> >  v2:
> > 
> >  - Updated the description suggested by tglx
> > 
> >  - Corrected the return value from -EIO to -EPERM when the userspace can't set the affinity
> > 
> >  kernel/irq/proc.c | 10 +++++++---
> > 
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> >  diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
> > 
> >  index 8cccdf40725a..7b3a4c92d148 100644
> > 
> >  --- a/kernel/irq/proc.c
> > 
> >  +++ b/kernel/irq/proc.c
> > 
> >  @@ -142,7 +142,7 @@ static ssize_t write_irq_affinity(int type, struct file *file,
> > 
> >  int err;
> > 
> >  
> > 
> >  if (!irq_can_set_affinity_usr(irq) || no_irq_affinity)
> > 
> >  - return -EIO;
> > 
> >  + return -EPERM;
> > 
> 
> I drop this hunk as it is unrelated to $subject. That want's to be a
> 
> separate patch. Documentation/process clearly states:
> 
>  Solve only one problem per patch.

Thank you for the reminder. I overlooked that single line change, which is indeed unrelated to the subject. I'll send it as a separate patch.

> Thanks,
> 
>  tglx
>

[tip: irq/core] genirq/procfs: Correctly set file permissions for affinity control files

[tip-bot2 for Jeff Xie]

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     0b39441eaab8bedcba1129776ec85178d4d0d9fb
Gitweb:        https://git.kernel.org/tip/0b39441eaab8bedcba1129776ec85178d4d0d9fb
Author:        Jeff Xie <jeff.xie@linux.dev>
AuthorDate:    Sun, 25 Aug 2024 21:19:11 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 26 Aug 2024 14:00:25 +02:00

genirq/procfs: Correctly set file permissions for affinity control files

The kernel already knows at the time of interrupt allocation whether
affinity of an interrupt can be controlled by userspace or not.

It still creates all related procfs control files with read/write
permissions. That's inconsistent and non-intuitive for system
administrators and tools.

Therefore set the file permissions to read-only for such interrupts.

[ tglx: Massage change log ]

Signed-off-by: Jeff Xie <jeff.xie@linux.dev>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20240825131911.107119-1-jeff.xie@linux.dev
---
 kernel/irq/proc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 8cccdf4..dcf8190 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -340,6 +340,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 	static DEFINE_MUTEX(register_lock);
 	void __maybe_unused *irqp = (void *)(unsigned long) irq;
 	char name [MAX_NAMELEN];
+	umode_t umode = S_IRUGO;
 
 	if (!root_irq_dir || (desc->irq_data.chip == &no_irq_chip))
 		return;
@@ -362,8 +363,11 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 		goto out_unlock;
 
 #ifdef CONFIG_SMP
+	if (irq_can_set_affinity_usr(desc->irq_data.irq))
+		umode |= S_IWUSR;
+
 	/* create /proc/irq/<irq>/smp_affinity */
-	proc_create_data("smp_affinity", 0644, desc->dir,
+	proc_create_data("smp_affinity", umode, desc->dir,
 			 &irq_affinity_proc_ops, irqp);
 
 	/* create /proc/irq/<irq>/affinity_hint */
@@ -371,7 +375,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 			irq_affinity_hint_proc_show, irqp);
 
 	/* create /proc/irq/<irq>/smp_affinity_list */
-	proc_create_data("smp_affinity_list", 0644, desc->dir,
+	proc_create_data("smp_affinity_list", umode, desc->dir,
 			 &irq_affinity_list_proc_ops, irqp);
 
 	proc_create_single_data("node", 0444, desc->dir, irq_node_proc_show,

Re: [PATCH v2] genirq: procfs: Make smp_affinity read-only for interrupts that userspace can't set

[kernel test robot]

Hi Jeff,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/irq/core]
[also build test WARNING on linus/master v6.11-rc5 next-20240826]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeff-Xie/genirq-procfs-Make-smp_affinity-read-only-for-interrupts-that-userspace-can-t-set/20240826-153926
base:   tip/irq/core
patch link:    https://lore.kernel.org/r/20240825131911.107119-1-jeff.xie%40linux.dev
patch subject: [PATCH v2] genirq: procfs: Make smp_affinity read-only for interrupts that userspace can't set
config: openrisc-allnoconfig (https://download.01.org/0day-ci/archive/20240827/202408271311.szIyk0et-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240827/202408271311.szIyk0et-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408271311.szIyk0et-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/irq/proc.c: In function 'register_irq_proc':
>> kernel/irq/proc.c:343:17: warning: unused variable 'umode' [-Wunused-variable]
     343 |         umode_t umode = S_IRUGO;
         |                 ^~~~~


vim +/umode +343 kernel/irq/proc.c

   337	
   338	void register_irq_proc(unsigned int irq, struct irq_desc *desc)
   339	{
   340		static DEFINE_MUTEX(register_lock);
   341		void __maybe_unused *irqp = (void *)(unsigned long) irq;
   342		char name [MAX_NAMELEN];
 > 343		umode_t umode = S_IRUGO;
   344	
   345		if (!root_irq_dir || (desc->irq_data.chip == &no_irq_chip))
   346			return;
   347	
   348		/*
   349		 * irq directories are registered only when a handler is
   350		 * added, not when the descriptor is created, so multiple
   351		 * tasks might try to register at the same time.
   352		 */
   353		mutex_lock(&register_lock);
   354	
   355		if (desc->dir)
   356			goto out_unlock;
   357	
   358		sprintf(name, "%d", irq);
   359	
   360		/* create /proc/irq/1234 */
   361		desc->dir = proc_mkdir(name, root_irq_dir);
   362		if (!desc->dir)
   363			goto out_unlock;
   364	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[tip: irq/core] genirq/procfs: Correctly set file permissions for affinity control files

[tip-bot2 for Jeff Xie]

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     413abc4d0f1a1a19713db34ee689609856ffea0e
Gitweb:        https://git.kernel.org/tip/413abc4d0f1a1a19713db34ee689609856ffea0e
Author:        Jeff Xie <jeff.xie@linux.dev>
AuthorDate:    Sun, 25 Aug 2024 21:19:11 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 27 Aug 2024 12:16:40 +02:00

genirq/procfs: Correctly set file permissions for affinity control files

The kernel already knows at the time of interrupt allocation whether
affinity of an interrupt can be controlled by userspace or not.

It still creates all related procfs control files with read/write
permissions. That's inconsistent and non-intuitive for system
administrators and tools.

Therefore set the file permissions to read-only for such interrupts.

[ tglx: Massage change log ]

Signed-off-by: Jeff Xie <jeff.xie@linux.dev>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/all/20240825131911.107119-1-jeff.xie@linux.dev
---
 kernel/irq/proc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 8cccdf4..dcf8190 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -340,6 +340,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 	static DEFINE_MUTEX(register_lock);
 	void __maybe_unused *irqp = (void *)(unsigned long) irq;
 	char name [MAX_NAMELEN];
+	umode_t umode = S_IRUGO;
 
 	if (!root_irq_dir || (desc->irq_data.chip == &no_irq_chip))
 		return;
@@ -362,8 +363,11 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 		goto out_unlock;
 
 #ifdef CONFIG_SMP
+	if (irq_can_set_affinity_usr(desc->irq_data.irq))
+		umode |= S_IWUSR;
+
 	/* create /proc/irq/<irq>/smp_affinity */
-	proc_create_data("smp_affinity", 0644, desc->dir,
+	proc_create_data("smp_affinity", umode, desc->dir,
 			 &irq_affinity_proc_ops, irqp);
 
 	/* create /proc/irq/<irq>/affinity_hint */
@@ -371,7 +375,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 			irq_affinity_hint_proc_show, irqp);
 
 	/* create /proc/irq/<irq>/smp_affinity_list */
-	proc_create_data("smp_affinity_list", 0644, desc->dir,
+	proc_create_data("smp_affinity_list", umode, desc->dir,
 			 &irq_affinity_list_proc_ops, irqp);
 
 	proc_create_single_data("node", 0444, desc->dir, irq_node_proc_show,

[tip: irq/core] genirq/procfs: Correctly set file permissions for affinity control files

[tip-bot2 for Jeff Xie]

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     92f9d825b12fa3f6c14b42405489880d0694c96f
Gitweb:        https://git.kernel.org/tip/92f9d825b12fa3f6c14b42405489880d0694c96f
Author:        Jeff Xie <jeff.xie@linux.dev>
AuthorDate:    Sun, 25 Aug 2024 21:19:11 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Tue, 27 Aug 2024 13:54:15 +02:00

genirq/procfs: Correctly set file permissions for affinity control files

The kernel already knows at the time of interrupt allocation whether
affinity of an interrupt can be controlled by userspace or not.

It still creates all related procfs control files with read/write
permissions. That's inconsistent and non-intuitive for system
administrators and tools.

Therefore set the file permissions to read-only for such interrupts.

[ tglx: Massage change log ]

Signed-off-by: Jeff Xie <jeff.xie@linux.dev>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20240825131911.107119-1-jeff.xie@linux.dev
---
 kernel/irq/proc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 8cccdf4..dcf8190 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -340,6 +340,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 	static DEFINE_MUTEX(register_lock);
 	void __maybe_unused *irqp = (void *)(unsigned long) irq;
 	char name [MAX_NAMELEN];
+	umode_t umode = S_IRUGO;
 
 	if (!root_irq_dir || (desc->irq_data.chip == &no_irq_chip))
 		return;
@@ -362,8 +363,11 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 		goto out_unlock;
 
 #ifdef CONFIG_SMP
+	if (irq_can_set_affinity_usr(desc->irq_data.irq))
+		umode |= S_IWUSR;
+
 	/* create /proc/irq/<irq>/smp_affinity */
-	proc_create_data("smp_affinity", 0644, desc->dir,
+	proc_create_data("smp_affinity", umode, desc->dir,
 			 &irq_affinity_proc_ops, irqp);
 
 	/* create /proc/irq/<irq>/affinity_hint */
@@ -371,7 +375,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 			irq_affinity_hint_proc_show, irqp);
 
 	/* create /proc/irq/<irq>/smp_affinity_list */
-	proc_create_data("smp_affinity_list", 0644, desc->dir,
+	proc_create_data("smp_affinity_list", umode, desc->dir,
 			 &irq_affinity_list_proc_ops, irqp);
 
 	proc_create_single_data("node", 0444, desc->dir, irq_node_proc_show,

[tip: irq/core] genirq/proc: Correctly set file permissions for affinity control files

[tip-bot2 for Jeff Xie]

The following commit has been merged into the irq/core branch of tip:

Commit-ID:     c7718e5c76d49b5bb394265383ae51f766d5dd3a
Gitweb:        https://git.kernel.org/tip/c7718e5c76d49b5bb394265383ae51f766d5dd3a
Author:        Jeff Xie <jeff.xie@linux.dev>
AuthorDate:    Sun, 25 Aug 2024 21:19:11 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Thu, 29 Aug 2024 16:41:42 +02:00

genirq/proc: Correctly set file permissions for affinity control files

The kernel already knows at the time of interrupt allocation whether
affinity of an interrupt can be controlled by userspace or not.

It still creates all related procfs control files with read/write
permissions. That's inconsistent and non-intuitive for system
administrators and tools.

Therefore set the file permissions to read-only for such interrupts.

[ tglx: Massage change log, fixed UP build ]

Signed-off-by: Jeff Xie <jeff.xie@linux.dev>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20240825131911.107119-1-jeff.xie@linux.dev
---
 kernel/irq/proc.c |  9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 8cccdf4..9b3b12a 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -362,8 +362,13 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 		goto out_unlock;
 
 #ifdef CONFIG_SMP
+	umode_t umode = S_IRUGO;
+
+	if (irq_can_set_affinity_usr(desc->irq_data.irq))
+		umode |= S_IWUSR;
+
 	/* create /proc/irq/<irq>/smp_affinity */
-	proc_create_data("smp_affinity", 0644, desc->dir,
+	proc_create_data("smp_affinity", umode, desc->dir,
 			 &irq_affinity_proc_ops, irqp);
 
 	/* create /proc/irq/<irq>/affinity_hint */
@@ -371,7 +376,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 			irq_affinity_hint_proc_show, irqp);
 
 	/* create /proc/irq/<irq>/smp_affinity_list */
-	proc_create_data("smp_affinity_list", 0644, desc->dir,
+	proc_create_data("smp_affinity_list", umode, desc->dir,
 			 &irq_affinity_list_proc_ops, irqp);
 
 	proc_create_single_data("node", 0444, desc->dir, irq_node_proc_show,