set {msi_domain,syscore}_ops as __ro_after_init

PATCH Superseded Jess Frazelle 5 participants 14 messages Feb 10, 2017

Subsystems (6)

x86 Staging PCI IRQ +2 more subsystems

lore

Share & tools

lore

Jess Frazelle

author

2017-02-10 10:08 UTC #0 lore raw

cover set {msi_domain,syscore}_ops as __ro_after_init

This is an initial pass and sanity check to make sure this is correct. The changes have all been tested on x86.

Best,
Jess

Jess Frazelle

author

2017-02-10 10:08 UTC #1 lore raw

PATCH 1/4 irq: set {msi_domain,syscore}_ops as __ro_after_init

Marked msi_domain_ops structs as __ro_after_init when called only during init. Marked syscore_ops structs as __ro_after_init when register_syscore_ops was called only during init. Most of the caller functions were already annotated as __init.
This protects the data structure from accidental corruption.

Suggested-by Kees Cook <[email protected]>

Signed-off-by Jess Frazelle <[email protected]>

---
 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 2 +-
 kernel/irq/generic-chip.c                              | 2 +-
 kernel/irq/msi.c                                       | 2 +-
 kernel/irq/pm.c                                        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +1 -1

index 6b1cd574644f..0e2c1b5e13b7 100644
@@ -51,7 +51,7 @@ static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain,
 	return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info);
 }
 
-static struct msi_domain_ops its_fsl_mc_msi_ops = {
+static struct msi_domain_ops its_fsl_mc_msi_ops __ro_after_init = {
 	.msi_prepare = its_fsl_mc_msi_prepare,
 };

kernel/irq/generic-chip.c +1 -1

index ee32870079c9..cca63dbaabea 100644
@@ -623,7 +623,7 @@ static void irq_gc_shutdown(void)
 	}
 }
 
-static struct syscore_ops irq_gc_syscore_ops = {
+static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
 	.suspend = irq_gc_suspend,
 	.resume = irq_gc_resume,
 	.shutdown = irq_gc_shutdown,

kernel/irq/msi.c +1 -1

index ee230063f033..0e5b723f710f 100644
@@ -217,7 +217,7 @@ static int msi_domain_ops_check(struct irq_domain *domain,
 	return 0;
 }
 
-static struct msi_domain_ops msi_domain_ops_default = {
+static struct msi_domain_ops msi_domain_ops_default __ro_after_init = {
 	.get_hwirq	= msi_domain_ops_get_hwirq,
 	.msi_init	= msi_domain_ops_init,
 	.msi_check	= msi_domain_ops_check,

kernel/irq/pm.c +1 -1

index cea1de0161f1..d6b889bed323 100644
@@ -185,7 +185,7 @@ static void irq_pm_syscore_resume(void)
 	resume_irqs(true);
 }
 
-static struct syscore_ops irq_pm_syscore_ops = {
+static struct syscore_ops irq_pm_syscore_ops __ro_after_init = {
 	.resume		= irq_pm_syscore_resume,
 };

--
2.11.0

Laura Abbott

2017-02-10 20:26 UTC #2 lore raw

Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init

Quoted 20 lines · Jess Frazelle

Marked msi_domain_ops structs as __ro_after_init when called only during init. Marked syscore_ops structs as __ro_after_init when register_syscore_ops was called only during init. Most of the caller functions were already annotated as __init.
This protects the data structure from accidental corruption.
Suggested-by Kees Cook <[email protected]>
Signed-off-by Jess Frazelle <[email protected]>
---
 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 2 +-
 kernel/irq/generic-chip.c                              | 2 +-
 kernel/irq/msi.c                                       | 2 +-
 kernel/irq/pm.c                                        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +0 -0
index 6b1cd574644f..0e2c1b5e13b7 100644

drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain, · line 57

Quoted 8 lines

drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +1 -1

@@ -51,7 +51,7 @@ static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain,
 	return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info);
 }
 
-static struct msi_domain_ops its_fsl_mc_msi_ops = {
+static struct msi_domain_ops its_fsl_mc_msi_ops __ro_after_init = {
 	.msi_prepare = its_fsl_mc_msi_prepare,
 };

The staging change is probably better suited to its own patch since staging drivers tend to get handled differently.

Thanks,
Laura

Quoted 42 lines

kernel/irq/generic-chip.c +1 -1

index ee32870079c9..cca63dbaabea 100644
@@ -623,7 +623,7 @@ static void irq_gc_shutdown(void)
 	}
 }
 
-static struct syscore_ops irq_gc_syscore_ops = {
+static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
 	.suspend = irq_gc_suspend,
 	.resume = irq_gc_resume,
 	.shutdown = irq_gc_shutdown,

kernel/irq/msi.c +1 -1

index ee230063f033..0e5b723f710f 100644
@@ -217,7 +217,7 @@ static int msi_domain_ops_check(struct irq_domain *domain,
 	return 0;
 }
 
-static struct msi_domain_ops msi_domain_ops_default = {
+static struct msi_domain_ops msi_domain_ops_default __ro_after_init = {
 	.get_hwirq	= msi_domain_ops_get_hwirq,
 	.msi_init	= msi_domain_ops_init,
 	.msi_check	= msi_domain_ops_check,

kernel/irq/pm.c +1 -2

index cea1de0161f1..d6b889bed323 100644
@@ -185,7 +185,7 @@ static void irq_pm_syscore_resume(void)
 	resume_irqs(true);
 }
 
-static struct syscore_ops irq_pm_syscore_ops = {
+static struct syscore_ops irq_pm_syscore_ops __ro_after_init = {
 	.resume		= irq_pm_syscore_resume,
 };
 
--

2.11.0

Jessica Frazelle

author

2017-02-10 20:34 UTC #3 lore raw

Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init

Got it, thanks!

Quoted 80 lines · Laura Abbott

Quoted 20 lines · Jess Frazelle
Marked msi_domain_ops structs as __ro_after_init when called only during init. Marked syscore_ops structs as __ro_after_init when register_syscore_ops was called only during init. Most of the caller functions were already annotated as __init.
This protects the data structure from accidental corruption.
Suggested-by Kees Cook <[email protected]>
Signed-off-by Jess Frazelle <[email protected]>
---
 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 2 +-
 kernel/irq/generic-chip.c                              | 2 +-
 kernel/irq/msi.c                                       | 2 +-
 kernel/irq/pm.c                                        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +0 -0
index 6b1cd574644f..0e2c1b5e13b7 100644
drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain, · line 57
Quoted 8 lines
drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +1 -1
@@ -51,7 +51,7 @@ static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain,
      return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info);
 }
 
-static struct msi_domain_ops its_fsl_mc_msi_ops = {
+static struct msi_domain_ops its_fsl_mc_msi_ops __ro_after_init = {
      .msi_prepare = its_fsl_mc_msi_prepare,
 };
 
The staging change is probably better suited to its own patch since staging drivers tend to get handled differently.
Thanks,
Laura
Quoted 42 lines
kernel/irq/generic-chip.c +1 -1
index ee32870079c9..cca63dbaabea 100644
@@ -623,7 +623,7 @@ static void irq_gc_shutdown(void)
      }
 }
 
-static struct syscore_ops irq_gc_syscore_ops = {
+static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
      .suspend = irq_gc_suspend,
      .resume = irq_gc_resume,
      .shutdown = irq_gc_shutdown,
kernel/irq/msi.c +1 -1
index ee230063f033..0e5b723f710f 100644
@@ -217,7 +217,7 @@ static int msi_domain_ops_check(struct irq_domain *domain,
      return 0;
 }
 
-static struct msi_domain_ops msi_domain_ops_default = {
+static struct msi_domain_ops msi_domain_ops_default __ro_after_init = {
      .get_hwirq      = msi_domain_ops_get_hwirq,
      .msi_init       = msi_domain_ops_init,
      .msi_check      = msi_domain_ops_check,
kernel/irq/pm.c +1 -2
index cea1de0161f1..d6b889bed323 100644
@@ -185,7 +185,7 @@ static void irq_pm_syscore_resume(void)
      resume_irqs(true);
 }
 
-static struct syscore_ops irq_pm_syscore_ops = {
+static struct syscore_ops irq_pm_syscore_ops __ro_after_init = {
      .resume         = irq_pm_syscore_resume,
 };
 
--
2.11.0

Kees Cook

2017-02-10 21:55 UTC #4 lore raw

Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init

In reply to Jess Frazelle

Quoted 59 lines · Jess Frazelle

Suggested-by Kees Cook <[email protected]>

Signed-off-by Jess Frazelle <[email protected]>

---
 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 2 +-
 kernel/irq/generic-chip.c                              | 2 +-
 kernel/irq/msi.c                                       | 2 +-
 kernel/irq/pm.c                                        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +1 -1

index 6b1cd574644f..0e2c1b5e13b7 100644
@@ -51,7 +51,7 @@ static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain,
        return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info);
 }
 
-static struct msi_domain_ops its_fsl_mc_msi_ops = {
+static struct msi_domain_ops its_fsl_mc_msi_ops __ro_after_init = {
        .msi_prepare = its_fsl_mc_msi_prepare,
 };

kernel/irq/generic-chip.c +1 -1

index ee32870079c9..cca63dbaabea 100644
@@ -623,7 +623,7 @@ static void irq_gc_shutdown(void)
        }
 }
 
-static struct syscore_ops irq_gc_syscore_ops = {
+static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
        .suspend = irq_gc_suspend,
        .resume = irq_gc_resume,
        .shutdown = irq_gc_shutdown,

kernel/irq/msi.c +1 -1

index ee230063f033..0e5b723f710f 100644
@@ -217,7 +217,7 @@ static int msi_domain_ops_check(struct irq_domain *domain,
        return 0;
 }
 
-static struct msi_domain_ops msi_domain_ops_default = {
+static struct msi_domain_ops msi_domain_ops_default __ro_after_init = {
        .get_hwirq      = msi_domain_ops_get_hwirq,
        .msi_init       = msi_domain_ops_init,
        .msi_check      = msi_domain_ops_check,

kernel/irq/pm.c +0 -0

index cea1de0161f1..d6b889bed323 100644

kernel/irq/pm.c static void irq_pm_syscore_resume(void) · line 190

Quoted 7 lines

kernel/irq/pm.c +1 -1

@@ -185,7 +185,7 @@ static void irq_pm_syscore_resume(void)
        resume_irqs(true);
 }
 
-static struct syscore_ops irq_pm_syscore_ops = {
+static struct syscore_ops irq_pm_syscore_ops __ro_after_init = {
        .resume         = irq_pm_syscore_resume,
 };

Cool! How'd you end up choosing these? Did you just go looking for one-sided initializations? (i.e. register_syscore_ops() without unregister_syscore_ops() call?)

(It may help the commit message to explicitly state that unregister_syscore_ops() is never called on these ops.)

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Jessica Frazelle

author

2017-02-10 22:02 UTC #5 lore raw

Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init

Quoted 63 lines · Kees Cook

Quoted 62 lines · Jess Frazelle

Suggested-by Kees Cook <[email protected]>

Signed-off-by Jess Frazelle <[email protected]>

---
 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 2 +-
 kernel/irq/generic-chip.c                              | 2 +-
 kernel/irq/msi.c                                       | 2 +-
 kernel/irq/pm.c                                        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +1 -1

index 6b1cd574644f..0e2c1b5e13b7 100644
@@ -51,7 +51,7 @@ static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain,
        return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info);
 }
 
-static struct msi_domain_ops its_fsl_mc_msi_ops = {
+static struct msi_domain_ops its_fsl_mc_msi_ops __ro_after_init = {
        .msi_prepare = its_fsl_mc_msi_prepare,
 };

kernel/irq/generic-chip.c +1 -1

index ee32870079c9..cca63dbaabea 100644
@@ -623,7 +623,7 @@ static void irq_gc_shutdown(void)
        }
 }
 
-static struct syscore_ops irq_gc_syscore_ops = {
+static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
        .suspend = irq_gc_suspend,
        .resume = irq_gc_resume,
        .shutdown = irq_gc_shutdown,

kernel/irq/msi.c +1 -1

index ee230063f033..0e5b723f710f 100644
@@ -217,7 +217,7 @@ static int msi_domain_ops_check(struct irq_domain *domain,
        return 0;
 }
 
-static struct msi_domain_ops msi_domain_ops_default = {
+static struct msi_domain_ops msi_domain_ops_default __ro_after_init = {
        .get_hwirq      = msi_domain_ops_get_hwirq,
        .msi_init       = msi_domain_ops_init,
        .msi_check      = msi_domain_ops_check,

kernel/irq/pm.c +0 -0

index cea1de0161f1..d6b889bed323 100644
@@ -185,7 +185,7 @@ static void irq_pm_syscore_resume(void)
        resume_irqs(true);
 }

Quoted 8 lines

kernel/irq/pm.c static void irq_pm_syscore_resume(void) · line 190
Quoted 4 lines
kernel/irq/pm.c +1 -1
-static struct syscore_ops irq_pm_syscore_ops = {
+static struct syscore_ops irq_pm_syscore_ops __ro_after_init = {
        .resume         = irq_pm_syscore_resume,
 };
Cool! How'd you end up choosing these? Did you just go looking for one-sided initializations? (i.e. register_syscore_ops() without unregister_syscore_ops() call?)

I first made the mistake of making them all __ro_after_init and I broke the kvm module, so I had to track down how that happened since I didn't touch that code path... I ended up digging a bit into the call stack that ultimately leads to these and this is my initial "safe set". I am still digging into the others. But almost all the functions that call syscore_ops structs were already marked as __init.

Quoted 3 lines

(It may help the commit message to explicitly state that unregister_syscore_ops() is never called on these ops.)

Will fix!

Quoted 6 lines

-Kees
--
Kees Cook
Pixel Security

Signature · 3 lines

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC  511E 18F3 685C 0022 BFF3
pgp.mit.edu

Kees Cook

2017-02-10 23:15 UTC #6 lore raw

Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init

Quoted 73 lines · Jessica Frazelle

Quoted 63 lines · Kees Cook

Quoted 62 lines · Jess Frazelle

Suggested-by Kees Cook <[email protected]>

Signed-off-by Jess Frazelle <[email protected]>

---
 drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 2 +-
 kernel/irq/generic-chip.c                              | 2 +-
 kernel/irq/msi.c                                       | 2 +-
 kernel/irq/pm.c                                        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c +1 -1

index 6b1cd574644f..0e2c1b5e13b7 100644
@@ -51,7 +51,7 @@ static int its_fsl_mc_msi_prepare(struct irq_domain *msi_domain,
        return msi_info->ops->msi_prepare(msi_domain->parent, dev, nvec, info);
 }
 
-static struct msi_domain_ops its_fsl_mc_msi_ops = {
+static struct msi_domain_ops its_fsl_mc_msi_ops __ro_after_init = {
        .msi_prepare = its_fsl_mc_msi_prepare,
 };

kernel/irq/generic-chip.c +1 -1

index ee32870079c9..cca63dbaabea 100644
@@ -623,7 +623,7 @@ static void irq_gc_shutdown(void)
        }
 }
 
-static struct syscore_ops irq_gc_syscore_ops = {
+static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
        .suspend = irq_gc_suspend,
        .resume = irq_gc_resume,
        .shutdown = irq_gc_shutdown,

kernel/irq/msi.c +1 -1

index ee230063f033..0e5b723f710f 100644
@@ -217,7 +217,7 @@ static int msi_domain_ops_check(struct irq_domain *domain,
        return 0;
 }
 
-static struct msi_domain_ops msi_domain_ops_default = {
+static struct msi_domain_ops msi_domain_ops_default __ro_after_init = {
        .get_hwirq      = msi_domain_ops_get_hwirq,
        .msi_init       = msi_domain_ops_init,
        .msi_check      = msi_domain_ops_check,

kernel/irq/pm.c +0 -0

index cea1de0161f1..d6b889bed323 100644
@@ -185,7 +185,7 @@ static void irq_pm_syscore_resume(void)
        resume_irqs(true);
 }

Quoted 8 lines

kernel/irq/pm.c static void irq_pm_syscore_resume(void) · line 190
Quoted 4 lines
kernel/irq/pm.c +1 -1
-static struct syscore_ops irq_pm_syscore_ops = {
+static struct syscore_ops irq_pm_syscore_ops __ro_after_init = {
        .resume         = irq_pm_syscore_resume,
 };
Cool! How'd you end up choosing these? Did you just go looking for one-sided initializations? (i.e. register_syscore_ops() without unregister_syscore_ops() call?)

Quoted 6 lines

I first made the mistake of making them all __ro_after_init and I broke the kvm module, so I had to track down how that happened since I didn't touch that code path... I ended up digging a bit into the call stack that ultimately leads to these and this is my initial "safe set". I am still digging into the others. But almost all the functions that call syscore_ops structs were already marked as __init.

Right, the danger is if something outside of __init tries to change the structure, but none of these seem to ever have an UNregister call made against them. :)

Quoted 4 lines

Quoted 2 lines
(It may help the commit message to explicitly state that unregister_syscore_ops() is never called on these ops.)
Will fix!

Awesome. :)

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Jess Frazelle

author

2017-02-10 10:09 UTC #7 lore raw

PATCH 2/4 pci: set msi_domain_ops as __ro_after_init

In reply to Jess Frazelle

Marked msi_domain_ops structs as __ro_after_init when called only during init. This protects the data structure from accidental corruption.

Suggested-by Kees Cook <[email protected]>

Signed-off-by Jess Frazelle <[email protected]>

---
 drivers/pci/host/pci-hyperv.c | 2 +-
 drivers/pci/host/vmd.c        | 2 +-
 drivers/pci/msi.c             | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

drivers/pci/host/pci-hyperv.c +1 -1

index 3efcc7bdc5fb..f05b93689d8f 100644
@@ -958,7 +958,7 @@ static irq_hw_number_t hv_msi_domain_ops_get_hwirq(struct msi_domain_info *info,
 	return arg->msi_hwirq;
 }
 
-static struct msi_domain_ops hv_msi_ops = {
+static struct msi_domain_ops hv_msi_ops __ro_after_init = {
 	.get_hwirq	= hv_msi_domain_ops_get_hwirq,
 	.msi_prepare	= pci_msi_prepare,
 	.set_desc	= pci_msi_set_desc,

drivers/pci/host/vmd.c +1 -1

index 18ef1a93c10a..152c461538e4 100644
@@ -253,7 +253,7 @@ static void vmd_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
 	arg->desc = desc;
 }
 
-static struct msi_domain_ops vmd_msi_domain_ops = {
+static struct msi_domain_ops vmd_msi_domain_ops __ro_after_init = {
 	.get_hwirq	= vmd_get_hwirq,
 	.msi_init	= vmd_msi_init,
 	.msi_free	= vmd_msi_free,

drivers/pci/msi.c +1 -1

index 50c5003295ca..93141d5e2d1c 100644
@@ -1413,7 +1413,7 @@ static void pci_msi_domain_set_desc(msi_alloc_info_t *arg,
 #define pci_msi_domain_set_desc		NULL
 #endif
 
-static struct msi_domain_ops pci_msi_domain_ops_default = {
+static struct msi_domain_ops pci_msi_domain_ops_default __ro_after_init = {
 	.set_desc	= pci_msi_domain_set_desc,
 	.msi_check	= pci_msi_domain_check_cap,
 	.handle_error	= pci_msi_domain_handle_error,

--
2.11.0

Jess Frazelle

author

2017-02-10 10:09 UTC #8 lore raw

PATCH 3/4 x86: set msi_domain_ops as __ro_after_init

In reply to Jess Frazelle

Marked msi_domain_ops structs as __ro_after_init when called only during init. This protects the data structure from accidental corruption.

Suggested-by Kees Cook <[email protected]>

Signed-off-by Jess Frazelle <[email protected]>

---
 arch/x86/kernel/apic/msi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

arch/x86/kernel/apic/msi.c +3 -3

index 015bbf30e3e3..27783a1e7166 100644
@@ -121,7 +121,7 @@ void pci_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
 }
 EXPORT_SYMBOL_GPL(pci_msi_set_desc);
 
-static struct msi_domain_ops pci_msi_domain_ops = {
+static struct msi_domain_ops pci_msi_domain_ops __ro_after_init = {
 	.get_hwirq	= pci_msi_get_hwirq,
 	.msi_prepare	= pci_msi_prepare,
 	.set_desc	= pci_msi_set_desc,

@@ -207,7 +207,7 @@ static int dmar_msi_init(struct irq_domain *domain,
 	return 0;
 }
 
-static struct msi_domain_ops dmar_msi_domain_ops = {
+static struct msi_domain_ops dmar_msi_domain_ops __ro_after_init = {
 	.get_hwirq	= dmar_msi_get_hwirq,
 	.msi_init	= dmar_msi_init,
 };

@@ -304,7 +304,7 @@ static void hpet_msi_free(struct irq_domain *domain,
 	irq_clear_status_flags(virq, IRQ_MOVE_PCNTXT);
 }
 
-static struct msi_domain_ops hpet_msi_domain_ops = {
+static struct msi_domain_ops hpet_msi_domain_ops __ro_after_init = {
 	.get_hwirq	= hpet_msi_get_hwirq,
 	.msi_init	= hpet_msi_init,
 	.msi_free	= hpet_msi_free,

--
2.11.0

Jess Frazelle

author

2017-02-10 10:09 UTC #9 lore raw

PATCH 4/4 time: mark syscore_ops as __ro_after_init

In reply to Jess Frazelle

Marked syscore_ops structs as __ro_after_init when register_syscore_ops was called only during init. Most of the caller functions were already annotated as __init.

Suggested-by Kees Cook <[email protected]>

Signed-off-by Jess Frazelle <[email protected]>

---
 kernel/time/sched_clock.c | 2 +-
 kernel/time/timekeeping.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

kernel/time/sched_clock.c +1 -1

index a26036d37a38..5df2fc07300b 100644
@@ -289,7 +289,7 @@ static void sched_clock_resume(void)
 	rd->read_sched_clock = cd.actual_read_sched_clock;
 }
 
-static struct syscore_ops sched_clock_ops = {
+static struct syscore_ops sched_clock_ops __ro_after_init = {
 	.suspend	= sched_clock_suspend,
 	.resume		= sched_clock_resume,
 };

kernel/time/timekeeping.c +1 -1

index db087d7e106d..467e3021723a 100644
@@ -1756,7 +1756,7 @@ int timekeeping_suspend(void)
 }
 
 /* sysfs resume/suspend bits for timekeeping */
-static struct syscore_ops timekeeping_syscore_ops = {
+static struct syscore_ops timekeeping_syscore_ops __ro_after_init = {
 	.resume		= timekeeping_resume,
 	.suspend	= timekeeping_suspend,
 };

--
2.11.0

Rik van Riel

Acked-by on v1 4/4

2017-02-10 13:40 UTC #10 lore raw

Re: [kernel-hardening] [PATCH 4/4] time: mark syscore_ops as __ro_after_init

Quoted 8 lines · Jess Frazelle

Marked syscore_ops structs as __ro_after_init when register_syscore_ops was
called only during init. Most of the caller functions were already annotated as
__init.
Suggested-by Kees Cook <[email protected]>
Signed-off-by Jess Frazelle <[email protected]>

Acked-by Rik van Riel <[email protected]>

Anisse Astier

2017-02-10 10:23 UTC #11 lore raw

In reply to Jess Frazelle

Hi Jess,

Quoted 2 lines · Jess Frazelle

This is an initial pass and sanity check to make sure this is correct. The changes have all been tested on x86.

May I suggested for the next iteration that you Cc: the maintainers/lists for each affected subsystem ?

Regards,

Anisse

Jessica Frazelle

author

2017-02-10 10:25 UTC #12 lore raw

Thanks, ya I had planned on it, just didn't want to do that if this was wrong :)

Quoted 13 lines · Anisse Astier

Hi Jess,
Quoted 2 lines · Jess Frazelle
This is an initial pass and sanity check to make sure this is correct. The changes have all been tested on x86.
May I suggested for the next iteration that you Cc: the maintainers/lists for each affected subsystem ?
Regards,
Anisse

Signature · 3 lines

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC  511E 18F3 685C 0022 BFF3
pgp.mit.edu

Laura Abbott

2017-02-10 20:27 UTC #13 lore raw

In reply to Jess Frazelle

Quoted 6 lines · Jess Frazelle

This is an initial pass and sanity check to make sure this is correct. The changes have all been tested on x86.
Best,
Jess

Looks good to me. I also boot tested this on arm64 as well. I'll give an actual Tested-by when you send the follow on series.

Thanks,
Laura