[PATCH 0/5] Initial steps to constify irq_chip

[PATCH 0/5] Initial steps to constify irq_chip

[Christophe JAILLET]

struct irq_chip holds many function pointers and so, is a good
candidate for constification.


Some steps to constify these structures have already been done in 2022
with commit 393e1280f765 ("genirq: Allow irq_chip registration functions
to take a const irq_chip") and ef6e5d61eb7a ("genirq: Allow
irq_set_chip_handler_name_locked() to take a const irq_chip")

However these commits introduce some ugly casting to store some const
parameters into non-const struct members. [1] [2]


The goal of this serie is to try to go further, and eventually cleanly
constify irq_chip. As a consequence, it will increase the overall
security, because of the constification of some function pointers.

Based on elixir, it is referenced in 565 files. [3]
So, the road looks long because this structure is used in many places. 


It will need to constify the return value of irq_get_chip(),
irq_data_get_irq_chip() and irq_desc_get_chip() which are widely used.
It is expected that many other functions may need to be tweaked in order
to propagate this const qualifier where needed in call chains.

So, let start somewhere and see how far we can go :)


patch 1 and 2 only update the prototypes of some functions and should be
straightforward.

The 3 other patches update some variable declaration in kernel/irq/
I have split the patches according to the way a reference to a struct
irq_chip is gotten.


This serie may be invasive and touch many places to get rid of the ugly
castings, but as least patch 1 and 2 should IMHO be merged because they
are simple and go in the right direction.

The 3 other ones, and all the oher ones that will be needed, may not
need the effort.

Before going further, feedback is aoppreciated


CJ


[1]: https://elixir.bootlin.com/linux/v6.12-rc7/source/kernel/irq/chip.c#L49
[2]: https://elixir.bootlin.com/linux/v6.12-rc7/source/include/linux/irqdesc.h#L236
[3]: https://elixir.bootlin.com/linux/v6.12-rc7/A/ident/irq_chip


Christophe JAILLET (5):
  irqchip: Constify "struct irq_chip *" parameter in chained_irq_xxx()
    functions
  irqchip: Constify "struct irq_chip *" parameter in
    cond_unmask_eoi_irq()
  irqchip: Constify some direct access to irq_data->chip
  irqchip: Constify some irq_desc_get_chip() usage
  irqchip: Constify some irq_data_get_irq_chip() usage

 include/linux/irqchip/chained_irq.h |  4 ++--
 kernel/irq/chip.c                   | 25 +++++++++++++------------
 kernel/irq/cpuhotplug.c             |  2 +-
 kernel/irq/debugfs.c                |  2 +-
 kernel/irq/manage.c                 | 20 ++++++++++----------
 kernel/irq/migration.c              |  2 +-
 6 files changed, 28 insertions(+), 27 deletions(-)

-- 
2.47.0

[PATCH 1/5] irqchip: Constify "struct irq_chip *" parameter in chained_irq_xxx() functions

[Christophe JAILLET]

chained_irq_enter() and chained_irq_exit() don't modify their 'chip'
parameter.

So change the prototype of these functions to accept const struct irq_chip
pointer.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only
---
 include/linux/irqchip/chained_irq.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/irqchip/chained_irq.h b/include/linux/irqchip/chained_irq.h
index dd8b3c476666..f248d63acfca 100644
--- a/include/linux/irqchip/chained_irq.h
+++ b/include/linux/irqchip/chained_irq.h
@@ -13,7 +13,7 @@
  * Entry/exit functions for chained handlers where the primary IRQ chip
  * may implement either fasteoi or level-trigger flow control.
  */
-static inline void chained_irq_enter(struct irq_chip *chip,
+static inline void chained_irq_enter(const struct irq_chip *chip,
 				     struct irq_desc *desc)
 {
 	/* FastEOI controllers require no action on entry. */
@@ -29,7 +29,7 @@ static inline void chained_irq_enter(struct irq_chip *chip,
 	}
 }
 
-static inline void chained_irq_exit(struct irq_chip *chip,
+static inline void chained_irq_exit(const struct irq_chip *chip,
 				    struct irq_desc *desc)
 {
 	if (chip->irq_eoi)
-- 
2.47.0

[PATCH 2/5] irqchip: Constify "struct irq_chip *" parameter in cond_unmask_eoi_irq()

[Christophe JAILLET]

cond_unmask_eoi_irq() doesn't modify its 'chip' parameter.

So change the prototype of this function to accept const struct irq_chip
pointer.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only
---
 kernel/irq/chip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 271e9139de77..502133dc3b9c 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -654,7 +654,8 @@ void handle_level_irq(struct irq_desc *desc)
 }
 EXPORT_SYMBOL_GPL(handle_level_irq);
 
-static void cond_unmask_eoi_irq(struct irq_desc *desc, struct irq_chip *chip)
+static void cond_unmask_eoi_irq(struct irq_desc *desc,
+				const struct irq_chip *chip)
 {
 	if (!(desc->istate & IRQS_ONESHOT)) {
 		chip->irq_eoi(&desc->irq_data);
-- 
2.47.0

[PATCH 3/5] irqchip: Constify some direct access to irq_data->chip

[Christophe JAILLET]

When the 'chip' member of struct irq_data will be turned into a const
struct irq_chip, the const qualifier will be needed for local variables
that keep a reference to this field.

So start to add some of these const qualifiers.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only
---
 kernel/irq/chip.c      |  8 ++++----
 kernel/irq/debugfs.c   |  2 +-
 kernel/irq/manage.c    | 10 +++++-----
 kernel/irq/migration.c |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 502133dc3b9c..eca39c4dd094 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -442,7 +442,7 @@ void unmask_irq(struct irq_desc *desc)
 
 void unmask_threaded_irq(struct irq_desc *desc)
 {
-	struct irq_chip *chip = desc->irq_data.chip;
+	const struct irq_chip *chip = desc->irq_data.chip;
 
 	if (chip->flags & IRQCHIP_EOI_THREADED)
 		chip->irq_eoi(&desc->irq_data);
@@ -687,7 +687,7 @@ static void cond_unmask_eoi_irq(struct irq_desc *desc,
  */
 void handle_fasteoi_irq(struct irq_desc *desc)
 {
-	struct irq_chip *chip = desc->irq_data.chip;
+	const struct irq_chip *chip = desc->irq_data.chip;
 
 	raw_spin_lock(&desc->lock);
 
@@ -1212,7 +1212,7 @@ void irq_cpu_offline(void)
  */
 void handle_fasteoi_ack_irq(struct irq_desc *desc)
 {
-	struct irq_chip *chip = desc->irq_data.chip;
+	const struct irq_chip *chip = desc->irq_data.chip;
 
 	raw_spin_lock(&desc->lock);
 
@@ -1263,7 +1263,7 @@ EXPORT_SYMBOL_GPL(handle_fasteoi_ack_irq);
  */
 void handle_fasteoi_mask_irq(struct irq_desc *desc)
 {
-	struct irq_chip *chip = desc->irq_data.chip;
+	const struct irq_chip *chip = desc->irq_data.chip;
 
 	raw_spin_lock(&desc->lock);
 	mask_ack_irq(desc);
diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index c6ffb97966be..c9c1c28be1a9 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -58,7 +58,7 @@ static const struct irq_bit_descr irqchip_flags[] = {
 static void
 irq_debug_show_chip(struct seq_file *m, struct irq_data *data, int ind)
 {
-	struct irq_chip *chip = data->chip;
+	const struct irq_chip *chip = data->chip;
 
 	if (!chip) {
 		seq_printf(m, "chip: None\n");
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index f0803d6bd296..e7f548bc976b 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -958,7 +958,7 @@ int can_request_irq(unsigned int irq, unsigned long irqflags)
 
 int __irq_set_trigger(struct irq_desc *desc, unsigned long flags)
 {
-	struct irq_chip *chip = desc->irq_data.chip;
+	const struct irq_chip *chip = desc->irq_data.chip;
 	int ret, unmask = 0;
 
 	if (!chip || !chip->irq_set_type) {
@@ -1407,7 +1407,7 @@ static int irq_setup_forced_threading(struct irqaction *new)
 static int irq_request_resources(struct irq_desc *desc)
 {
 	struct irq_data *d = &desc->irq_data;
-	struct irq_chip *c = d->chip;
+	const struct irq_chip *c = d->chip;
 
 	return c->irq_request_resources ? c->irq_request_resources(d) : 0;
 }
@@ -1415,7 +1415,7 @@ static int irq_request_resources(struct irq_desc *desc)
 static void irq_release_resources(struct irq_desc *desc)
 {
 	struct irq_data *d = &desc->irq_data;
-	struct irq_chip *c = d->chip;
+	const struct irq_chip *c = d->chip;
 
 	if (c->irq_release_resources)
 		c->irq_release_resources(d);
@@ -1440,7 +1440,7 @@ static bool irq_supports_nmi(struct irq_desc *desc)
 static int irq_nmi_setup(struct irq_desc *desc)
 {
 	struct irq_data *d = irq_desc_get_irq_data(desc);
-	struct irq_chip *c = d->chip;
+	const struct irq_chip *c = d->chip;
 
 	return c->irq_nmi_setup ? c->irq_nmi_setup(d) : -EINVAL;
 }
@@ -1448,7 +1448,7 @@ static int irq_nmi_setup(struct irq_desc *desc)
 static void irq_nmi_teardown(struct irq_desc *desc)
 {
 	struct irq_data *d = irq_desc_get_irq_data(desc);
-	struct irq_chip *c = d->chip;
+	const struct irq_chip *c = d->chip;
 
 	if (c->irq_nmi_teardown)
 		c->irq_nmi_teardown(d);
diff --git a/kernel/irq/migration.c b/kernel/irq/migration.c
index eb150afd671f..fff2d9cd0239 100644
--- a/kernel/irq/migration.c
+++ b/kernel/irq/migration.c
@@ -39,7 +39,7 @@ void irq_move_masked_irq(struct irq_data *idata)
 {
 	struct irq_desc *desc = irq_data_to_desc(idata);
 	struct irq_data *data = &desc->irq_data;
-	struct irq_chip *chip = data->chip;
+	const struct irq_chip *chip = data->chip;
 
 	if (likely(!irqd_is_setaffinity_pending(data)))
 		return;
-- 
2.47.0

[PATCH 4/5] irqchip: Constify some irq_desc_get_chip() usage

[Christophe JAILLET]

When irq_desc_get_chip() will return a const struct irq_chip, the const
qualifier will be needed for local variables that store the return value of
this function.

So start to add some of these const qualifiers.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only
---
 kernel/irq/chip.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index eca39c4dd094..d3acccf7e2e2 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -751,7 +751,7 @@ EXPORT_SYMBOL_GPL(handle_fasteoi_irq);
  */
 void handle_fasteoi_nmi(struct irq_desc *desc)
 {
-	struct irq_chip *chip = irq_desc_get_chip(desc);
+	const struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct irqaction *action = desc->action;
 	unsigned int irq = irq_desc_get_irq(desc);
 	irqreturn_t res;
@@ -849,7 +849,7 @@ EXPORT_SYMBOL(handle_edge_irq);
  */
 void handle_edge_eoi_irq(struct irq_desc *desc)
 {
-	struct irq_chip *chip = irq_desc_get_chip(desc);
+	const struct irq_chip *chip = irq_desc_get_chip(desc);
 
 	raw_spin_lock(&desc->lock);
 
@@ -894,7 +894,7 @@ void handle_edge_eoi_irq(struct irq_desc *desc)
  */
 void handle_percpu_irq(struct irq_desc *desc)
 {
-	struct irq_chip *chip = irq_desc_get_chip(desc);
+	const struct irq_chip *chip = irq_desc_get_chip(desc);
 
 	/*
 	 * PER CPU interrupts are not serialized. Do not touch
@@ -924,7 +924,7 @@ void handle_percpu_irq(struct irq_desc *desc)
  */
 void handle_percpu_devid_irq(struct irq_desc *desc)
 {
-	struct irq_chip *chip = irq_desc_get_chip(desc);
+	const struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct irqaction *action = desc->action;
 	unsigned int irq = irq_desc_get_irq(desc);
 	irqreturn_t res;
@@ -967,7 +967,7 @@ void handle_percpu_devid_irq(struct irq_desc *desc)
  */
 void handle_percpu_devid_fasteoi_nmi(struct irq_desc *desc)
 {
-	struct irq_chip *chip = irq_desc_get_chip(desc);
+	const struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct irqaction *action = desc->action;
 	unsigned int irq = irq_desc_get_irq(desc);
 	irqreturn_t res;
-- 
2.47.0

[PATCH 5/5] irqchip: Constify some irq_data_get_irq_chip() usage

[Christophe JAILLET]

When irq_data_get_irq_chip() will return a const struct irq_chip, the const
qualifier will be needed for local variables that store the return value of
this function.

So start to add some of these const qualifiers.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only
---
 kernel/irq/chip.c       |  4 ++--
 kernel/irq/cpuhotplug.c |  2 +-
 kernel/irq/manage.c     | 12 ++++++------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index d3acccf7e2e2..2d540884ca05 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1145,7 +1145,7 @@ EXPORT_SYMBOL_GPL(irq_modify_status);
 void irq_cpu_online(void)
 {
 	struct irq_desc *desc;
-	struct irq_chip *chip;
+	const struct irq_chip *chip;
 	unsigned long flags;
 	unsigned int irq;
 
@@ -1175,7 +1175,7 @@ void irq_cpu_online(void)
 void irq_cpu_offline(void)
 {
 	struct irq_desc *desc;
-	struct irq_chip *chip;
+	const struct irq_chip *chip;
 	unsigned long flags;
 	unsigned int irq;
 
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 15a7654eff68..dbfa7d63438f 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -53,7 +53,7 @@ static inline bool irq_needs_fixup(struct irq_data *d)
 static bool migrate_one_irq(struct irq_desc *desc)
 {
 	struct irq_data *d = irq_desc_get_irq_data(desc);
-	struct irq_chip *chip = irq_data_get_irq_chip(d);
+	const struct irq_chip *chip = irq_data_get_irq_chip(d);
 	bool maskchip = !irq_can_move_pcntxt(d) && !irqd_irq_masked(d);
 	const struct cpumask *affinity;
 	bool brokeaff = false;
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index e7f548bc976b..dfe184218cc2 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -207,7 +207,7 @@ void irq_set_thread_affinity(struct irq_desc *desc)
 static void irq_validate_effective_affinity(struct irq_data *data)
 {
 	const struct cpumask *m = irq_data_get_effective_affinity_mask(data);
-	struct irq_chip *chip = irq_data_get_irq_chip(data);
+	const struct irq_chip *chip = irq_data_get_irq_chip(data);
 
 	if (!cpumask_empty(m))
 		return;
@@ -225,7 +225,7 @@ int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
 {
 	struct cpumask *tmp_mask = this_cpu_ptr(&__tmp_mask);
 	struct irq_desc *desc = irq_data_to_desc(data);
-	struct irq_chip *chip = irq_data_get_irq_chip(data);
+	const struct irq_chip *chip = irq_data_get_irq_chip(data);
 	const struct cpumask  *prog_mask;
 	int ret;
 
@@ -353,7 +353,7 @@ static bool irq_set_affinity_deactivated(struct irq_data *data,
 int irq_set_affinity_locked(struct irq_data *data, const struct cpumask *mask,
 			    bool force)
 {
-	struct irq_chip *chip = irq_data_get_irq_chip(data);
+	const struct irq_chip *chip = irq_data_get_irq_chip(data);
 	struct irq_desc *desc = irq_data_to_desc(data);
 	int ret = 0;
 
@@ -658,7 +658,7 @@ int irq_set_vcpu_affinity(unsigned int irq, void *vcpu_info)
 	unsigned long flags;
 	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, 0);
 	struct irq_data *data;
-	struct irq_chip *chip;
+	const struct irq_chip *chip;
 	int ret = -ENOSYS;
 
 	if (!desc)
@@ -2799,7 +2799,7 @@ void teardown_percpu_nmi(unsigned int irq)
 int __irq_get_irqchip_state(struct irq_data *data, enum irqchip_irq_state which,
 			    bool *state)
 {
-	struct irq_chip *chip;
+	const struct irq_chip *chip;
 	int err = -EINVAL;
 
 	do {
@@ -2871,7 +2871,7 @@ int irq_set_irqchip_state(unsigned int irq, enum irqchip_irq_state which,
 {
 	struct irq_desc *desc;
 	struct irq_data *data;
-	struct irq_chip *chip;
+	const struct irq_chip *chip;
 	unsigned long flags;
 	int err = -EINVAL;
 
-- 
2.47.0

Re: [PATCH 4/5] irqchip: Constify some irq_desc_get_chip() usage

[Thomas Gleixner]

On Sun, Nov 17 2024 at 10:49, Christophe JAILLET wrote:
> When irq_desc_get_chip() will return a const struct irq_chip, the const
> qualifier will be needed for local variables that store the return value of
> this function.
>
> So start to add some of these const qualifiers.

Can you please create a coccinelle script which does a tree wide
conversion of all irq_desc_get_chip() and irq_data_get_irq_chip()
instances?

That script can be run at the end of the next merge window once instead
of having to chase 240+ places with individual patches.

Thanks,

        tglx