* [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init
@ 2017-02-10 10:08 Jess Frazelle
2017-02-10 10:08 ` [kernel-hardening] [PATCH 1/4] irq: " Jess Frazelle
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Jess Frazelle @ 2017-02-10 10:08 UTC (permalink / raw)
To: kernel-hardening
This is an initial pass and sanity check to make sure this is correct. The
changes have all been tested on x86.
Best,
Jess
^ permalink raw reply [flat|nested] 14+ messages in thread
* [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 10:08 [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle
@ 2017-02-10 10:08 ` Jess Frazelle
2017-02-10 20:26 ` Laura Abbott
2017-02-10 21:55 ` Kees Cook
2017-02-10 10:09 ` [kernel-hardening] [PATCH 2/4] pci: set msi_domain_ops " Jess Frazelle
` (4 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Jess Frazelle @ 2017-02-10 10:08 UTC (permalink / raw)
To: kernel-hardening; +Cc: 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 <keescook@chromium.org>
Signed-off-by: Jess Frazelle <me@jessfraz.com>
---
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(-)
diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
index 6b1cd574644f..0e2c1b5e13b7 100644
--- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
+++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
@@ -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,
};
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index ee32870079c9..cca63dbaabea 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -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,
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index ee230063f033..0e5b723f710f 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -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,
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index cea1de0161f1..d6b889bed323 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [kernel-hardening] [PATCH 2/4] pci: set msi_domain_ops as __ro_after_init
2017-02-10 10:08 [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle
2017-02-10 10:08 ` [kernel-hardening] [PATCH 1/4] irq: " Jess Frazelle
@ 2017-02-10 10:09 ` Jess Frazelle
2017-02-10 10:09 ` [kernel-hardening] [PATCH 3/4] x86: " Jess Frazelle
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Jess Frazelle @ 2017-02-10 10:09 UTC (permalink / raw)
To: kernel-hardening; +Cc: 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 <keescook@chromium.org>
Signed-off-by: Jess Frazelle <me@jessfraz.com>
---
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(-)
diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 3efcc7bdc5fb..f05b93689d8f 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -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,
diff --git a/drivers/pci/host/vmd.c b/drivers/pci/host/vmd.c
index 18ef1a93c10a..152c461538e4 100644
--- a/drivers/pci/host/vmd.c
+++ b/drivers/pci/host/vmd.c
@@ -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,
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 50c5003295ca..93141d5e2d1c 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [kernel-hardening] [PATCH 3/4] x86: set msi_domain_ops as __ro_after_init
2017-02-10 10:08 [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle
2017-02-10 10:08 ` [kernel-hardening] [PATCH 1/4] irq: " Jess Frazelle
2017-02-10 10:09 ` [kernel-hardening] [PATCH 2/4] pci: set msi_domain_ops " Jess Frazelle
@ 2017-02-10 10:09 ` Jess Frazelle
2017-02-10 10:09 ` [kernel-hardening] [PATCH 4/4] time: mark syscore_ops " Jess Frazelle
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Jess Frazelle @ 2017-02-10 10:09 UTC (permalink / raw)
To: kernel-hardening; +Cc: 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 <keescook@chromium.org>
Signed-off-by: Jess Frazelle <me@jessfraz.com>
---
arch/x86/kernel/apic/msi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index 015bbf30e3e3..27783a1e7166 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [kernel-hardening] [PATCH 4/4] time: mark syscore_ops as __ro_after_init
2017-02-10 10:08 [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle
` (2 preceding siblings ...)
2017-02-10 10:09 ` [kernel-hardening] [PATCH 3/4] x86: " Jess Frazelle
@ 2017-02-10 10:09 ` Jess Frazelle
2017-02-10 13:40 ` Rik van Riel
2017-02-10 10:23 ` [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops " Anisse Astier
2017-02-10 20:27 ` Laura Abbott
5 siblings, 1 reply; 14+ messages in thread
From: Jess Frazelle @ 2017-02-10 10:09 UTC (permalink / raw)
To: kernel-hardening; +Cc: 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 <keescook@chromium.org>
Signed-off-by: Jess Frazelle <me@jessfraz.com>
---
kernel/time/sched_clock.c | 2 +-
kernel/time/timekeeping.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index a26036d37a38..5df2fc07300b 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -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,
};
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index db087d7e106d..467e3021723a 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 10:08 [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle
` (3 preceding siblings ...)
2017-02-10 10:09 ` [kernel-hardening] [PATCH 4/4] time: mark syscore_ops " Jess Frazelle
@ 2017-02-10 10:23 ` Anisse Astier
2017-02-10 10:25 ` Jessica Frazelle
2017-02-10 20:27 ` Laura Abbott
5 siblings, 1 reply; 14+ messages in thread
From: Anisse Astier @ 2017-02-10 10:23 UTC (permalink / raw)
To: Jess Frazelle; +Cc: kernel-hardening
Hi Jess,
On Fri, Feb 10, 2017 at 02:08:58AM -0800, Jess Frazelle wrote:
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 10:23 ` [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops " Anisse Astier
@ 2017-02-10 10:25 ` Jessica Frazelle
0 siblings, 0 replies; 14+ messages in thread
From: Jessica Frazelle @ 2017-02-10 10:25 UTC (permalink / raw)
To: Anisse Astier; +Cc: Kernel Hardening
Thanks, ya I had planned on it, just didn't want to do that if this was wrong :)
On Fri, Feb 10, 2017 at 2:23 AM, Anisse Astier <anisse@astier.eu> wrote:
> Hi Jess,
>
> On Fri, Feb 10, 2017 at 02:08:58AM -0800, Jess Frazelle wrote:
>> 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
>
--
Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 4/4] time: mark syscore_ops as __ro_after_init
2017-02-10 10:09 ` [kernel-hardening] [PATCH 4/4] time: mark syscore_ops " Jess Frazelle
@ 2017-02-10 13:40 ` Rik van Riel
0 siblings, 0 replies; 14+ messages in thread
From: Rik van Riel @ 2017-02-10 13:40 UTC (permalink / raw)
To: Jess Frazelle, kernel-hardening
On Fri, 2017-02-10 at 02:09 -0800, Jess Frazelle wrote:
> 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 <keescook@chromium.org>
> Signed-off-by: Jess Frazelle <me@jessfraz.com>
Acked-by: Rik van Riel <riel@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 10:08 ` [kernel-hardening] [PATCH 1/4] irq: " Jess Frazelle
@ 2017-02-10 20:26 ` Laura Abbott
2017-02-10 20:34 ` Jessica Frazelle
2017-02-10 21:55 ` Kees Cook
1 sibling, 1 reply; 14+ messages in thread
From: Laura Abbott @ 2017-02-10 20:26 UTC (permalink / raw)
To: Jess Frazelle, kernel-hardening
On 02/10/2017 02:08 AM, Jess Frazelle wrote:
> 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 <keescook@chromium.org>
> Signed-off-by: Jess Frazelle <me@jessfraz.com>
> ---
> 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(-)
>
> diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> index 6b1cd574644f..0e2c1b5e13b7 100644
> --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> @@ -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
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index ee32870079c9..cca63dbaabea 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -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,
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index ee230063f033..0e5b723f710f 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -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,
> diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> index cea1de0161f1..d6b889bed323 100644
> --- a/kernel/irq/pm.c
> +++ b/kernel/irq/pm.c
> @@ -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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 10:08 [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle
` (4 preceding siblings ...)
2017-02-10 10:23 ` [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops " Anisse Astier
@ 2017-02-10 20:27 ` Laura Abbott
5 siblings, 0 replies; 14+ messages in thread
From: Laura Abbott @ 2017-02-10 20:27 UTC (permalink / raw)
To: Jess Frazelle, kernel-hardening
On 02/10/2017 02:08 AM, Jess Frazelle wrote:
> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 20:26 ` Laura Abbott
@ 2017-02-10 20:34 ` Jessica Frazelle
0 siblings, 0 replies; 14+ messages in thread
From: Jessica Frazelle @ 2017-02-10 20:34 UTC (permalink / raw)
To: Laura Abbott, Kernel Hardening
Got it, thanks!
On Fri, Feb 10, 2017 at 12:26 PM Laura Abbott <labbott@redhat.com> wrote:
>
> On 02/10/2017 02:08 AM, Jess Frazelle wrote:
> > 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 <keescook@chromium.org>
> > Signed-off-by: Jess Frazelle <me@jessfraz.com>
> > ---
> > 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(-)
> >
> > diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> > index 6b1cd574644f..0e2c1b5e13b7 100644
> > --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> > +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> > @@ -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
>
> > diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> > index ee32870079c9..cca63dbaabea 100644
> > --- a/kernel/irq/generic-chip.c
> > +++ b/kernel/irq/generic-chip.c
> > @@ -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,
> > diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> > index ee230063f033..0e5b723f710f 100644
> > --- a/kernel/irq/msi.c
> > +++ b/kernel/irq/msi.c
> > @@ -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,
> > diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> > index cea1de0161f1..d6b889bed323 100644
> > --- a/kernel/irq/pm.c
> > +++ b/kernel/irq/pm.c
> > @@ -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
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 10:08 ` [kernel-hardening] [PATCH 1/4] irq: " Jess Frazelle
2017-02-10 20:26 ` Laura Abbott
@ 2017-02-10 21:55 ` Kees Cook
2017-02-10 22:02 ` Jessica Frazelle
1 sibling, 1 reply; 14+ messages in thread
From: Kees Cook @ 2017-02-10 21:55 UTC (permalink / raw)
To: Jess Frazelle; +Cc: kernel-hardening
On Fri, Feb 10, 2017 at 2:08 AM, Jess Frazelle <me@jessfraz.com> wrote:
> 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 <keescook@chromium.org>
> Signed-off-by: Jess Frazelle <me@jessfraz.com>
> ---
> 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(-)
>
> diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> index 6b1cd574644f..0e2c1b5e13b7 100644
> --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> @@ -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,
> };
>
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index ee32870079c9..cca63dbaabea 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -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,
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index ee230063f033..0e5b723f710f 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -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,
> diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> index cea1de0161f1..d6b889bed323 100644
> --- a/kernel/irq/pm.c
> +++ b/kernel/irq/pm.c
> @@ -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
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 21:55 ` Kees Cook
@ 2017-02-10 22:02 ` Jessica Frazelle
2017-02-10 23:15 ` Kees Cook
0 siblings, 1 reply; 14+ messages in thread
From: Jessica Frazelle @ 2017-02-10 22:02 UTC (permalink / raw)
To: Kees Cook; +Cc: kernel-hardening
On Fri, Feb 10, 2017 at 1:55 PM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Feb 10, 2017 at 2:08 AM, Jess Frazelle <me@jessfraz.com> wrote:
>> 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 <keescook@chromium.org>
>> Signed-off-by: Jess Frazelle <me@jessfraz.com>
>> ---
>> 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(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
>> index 6b1cd574644f..0e2c1b5e13b7 100644
>> --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
>> +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
>> @@ -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,
>> };
>>
>> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
>> index ee32870079c9..cca63dbaabea 100644
>> --- a/kernel/irq/generic-chip.c
>> +++ b/kernel/irq/generic-chip.c
>> @@ -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,
>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
>> index ee230063f033..0e5b723f710f 100644
>> --- a/kernel/irq/msi.c
>> +++ b/kernel/irq/msi.c
>> @@ -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,
>> diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
>> index cea1de0161f1..d6b889bed323 100644
>> --- a/kernel/irq/pm.c
>> +++ b/kernel/irq/pm.c
>> @@ -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?)
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.
>
> (It may help the commit message to explicitly state that
> unregister_syscore_ops() is never called on these ops.)
Will fix!
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
--
Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kernel-hardening] [PATCH 1/4] irq: set {msi_domain,syscore}_ops as __ro_after_init
2017-02-10 22:02 ` Jessica Frazelle
@ 2017-02-10 23:15 ` Kees Cook
0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2017-02-10 23:15 UTC (permalink / raw)
To: Jessica Frazelle; +Cc: kernel-hardening
On Fri, Feb 10, 2017 at 2:02 PM, Jessica Frazelle <me@jessfraz.com> wrote:
> On Fri, Feb 10, 2017 at 1:55 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Fri, Feb 10, 2017 at 2:08 AM, Jess Frazelle <me@jessfraz.com> wrote:
>>> 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 <keescook@chromium.org>
>>> Signed-off-by: Jess Frazelle <me@jessfraz.com>
>>> ---
>>> 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(-)
>>>
>>> diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
>>> index 6b1cd574644f..0e2c1b5e13b7 100644
>>> --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
>>> +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
>>> @@ -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,
>>> };
>>>
>>> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
>>> index ee32870079c9..cca63dbaabea 100644
>>> --- a/kernel/irq/generic-chip.c
>>> +++ b/kernel/irq/generic-chip.c
>>> @@ -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,
>>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
>>> index ee230063f033..0e5b723f710f 100644
>>> --- a/kernel/irq/msi.c
>>> +++ b/kernel/irq/msi.c
>>> @@ -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,
>>> diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
>>> index cea1de0161f1..d6b889bed323 100644
>>> --- a/kernel/irq/pm.c
>>> +++ b/kernel/irq/pm.c
>>> @@ -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?)
>
> 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. :)
>> (It may help the commit message to explicitly state that
>> unregister_syscore_ops() is never called on these ops.)
>
> Will fix!
Awesome. :)
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-02-10 23:15 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-10 10:08 [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle
2017-02-10 10:08 ` [kernel-hardening] [PATCH 1/4] irq: " Jess Frazelle
2017-02-10 20:26 ` Laura Abbott
2017-02-10 20:34 ` Jessica Frazelle
2017-02-10 21:55 ` Kees Cook
2017-02-10 22:02 ` Jessica Frazelle
2017-02-10 23:15 ` Kees Cook
2017-02-10 10:09 ` [kernel-hardening] [PATCH 2/4] pci: set msi_domain_ops " Jess Frazelle
2017-02-10 10:09 ` [kernel-hardening] [PATCH 3/4] x86: " Jess Frazelle
2017-02-10 10:09 ` [kernel-hardening] [PATCH 4/4] time: mark syscore_ops " Jess Frazelle
2017-02-10 13:40 ` Rik van Riel
2017-02-10 10:23 ` [kernel-hardening] [PATCH 0/4] set {msi_domain,syscore}_ops " Anisse Astier
2017-02-10 10:25 ` Jessica Frazelle
2017-02-10 20:27 ` Laura Abbott
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.