* [PATCH] kernel: dma: Make CMA boot parameters __ro_after_init
@ 2019-10-12 12:29 Shyam Saini
2019-10-14 2:25 ` Nathan Chancellor
2019-10-14 14:33 ` Robin Murphy
0 siblings, 2 replies; 5+ messages in thread
From: Shyam Saini @ 2019-10-12 12:29 UTC (permalink / raw)
To: kernel-hardening
Cc: iommu, linux-kernel, linux-mm, Shyam Saini, Christoph Hellwig,
Marek Szyprowski, Robin Murphy, Matthew Wilcox,
Christopher Lameter, Kees Cook
This parameters are not changed after early boot.
By making them __ro_after_init will reduce any attack surface in the
kernel.
Link: https://lwn.net/Articles/676145/
Cc: Christoph Hellwig <hch@lst.de>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Christopher Lameter <cl@linux.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Shyam Saini <mayhs11saini@gmail.com>
---
kernel/dma/contiguous.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 69cfb4345388..1b689b1303cd 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -42,10 +42,10 @@ struct cma *dma_contiguous_default_area;
* Users, who want to set the size of global CMA area for their system
* should use cma= kernel parameter.
*/
-static const phys_addr_t size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
-static phys_addr_t size_cmdline = -1;
-static phys_addr_t base_cmdline;
-static phys_addr_t limit_cmdline;
+static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
+static phys_addr_t __ro_after_init size_cmdline = -1;
+static phys_addr_t __ro_after_init base_cmdline;
+static phys_addr_t __ro_after_init limit_cmdline;
static int __init early_cma(char *p)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] kernel: dma: Make CMA boot parameters __ro_after_init
2019-10-12 12:29 [PATCH] kernel: dma: Make CMA boot parameters __ro_after_init Shyam Saini
@ 2019-10-14 2:25 ` Nathan Chancellor
2019-10-16 10:11 ` Shyam Saini
2019-10-14 14:33 ` Robin Murphy
1 sibling, 1 reply; 5+ messages in thread
From: Nathan Chancellor @ 2019-10-14 2:25 UTC (permalink / raw)
To: Shyam Saini
Cc: kernel-hardening, iommu, linux-kernel, linux-mm,
Christoph Hellwig, Marek Szyprowski, Robin Murphy,
Matthew Wilcox, Christopher Lameter, Kees Cook,
clang-built-linux
On Sat, Oct 12, 2019 at 05:59:18PM +0530, Shyam Saini wrote:
> This parameters are not changed after early boot.
> By making them __ro_after_init will reduce any attack surface in the
> kernel.
>
> Link: https://lwn.net/Articles/676145/
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Christopher Lameter <cl@linux.com>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Shyam Saini <mayhs11saini@gmail.com>
> ---
> kernel/dma/contiguous.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 69cfb4345388..1b689b1303cd 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -42,10 +42,10 @@ struct cma *dma_contiguous_default_area;
> * Users, who want to set the size of global CMA area for their system
> * should use cma= kernel parameter.
> */
> -static const phys_addr_t size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> -static phys_addr_t size_cmdline = -1;
> -static phys_addr_t base_cmdline;
> -static phys_addr_t limit_cmdline;
> +static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
The 0day bot reported an issue with this change with clang:
https://groups.google.com/d/msgid/clang-built-linux/201910140334.nhultlt8%25lkp%40intel.com
kernel/dma/contiguous.c:46:36: error: 'size_cmdline' causes a section type conflict with 'size_bytes'
static phys_addr_t __ro_after_init size_cmdline = -1;
^
kernel/dma/contiguous.c:45:42: note: declared here
static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
^
kernel/dma/contiguous.c:47:36: error: 'base_cmdline' causes a section type conflict with 'size_bytes'
static phys_addr_t __ro_after_init base_cmdline;
^
kernel/dma/contiguous.c:45:42: note: declared here
static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
^
kernel/dma/contiguous.c:48:36: error: 'limit_cmdline' causes a section type conflict with 'size_bytes'
static phys_addr_t __ro_after_init limit_cmdline;
^
kernel/dma/contiguous.c:45:42: note: declared here
static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
^
3 errors generated.
The errors seem kind of cryptic at first but something that is const
should automatically be in the read only section, this part of the
commit seems unnecessary. Removing that part of the change fixes the error.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] kernel: dma: Make CMA boot parameters __ro_after_init
2019-10-14 2:25 ` Nathan Chancellor
@ 2019-10-16 10:11 ` Shyam Saini
0 siblings, 0 replies; 5+ messages in thread
From: Shyam Saini @ 2019-10-16 10:11 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Kernel Hardening, iommu, linux-kernel, linux-mm,
Christoph Hellwig, Marek Szyprowski, Robin Murphy,
Matthew Wilcox, Christopher Lameter, Kees Cook,
clang-built-linux
Hi Nathan,
On Mon, Oct 14, 2019 at 7:55 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Sat, Oct 12, 2019 at 05:59:18PM +0530, Shyam Saini wrote:
> > This parameters are not changed after early boot.
> > By making them __ro_after_init will reduce any attack surface in the
> > kernel.
> >
> > Link: https://lwn.net/Articles/676145/
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: Matthew Wilcox <willy@infradead.org>
> > Cc: Christopher Lameter <cl@linux.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Shyam Saini <mayhs11saini@gmail.com>
> > ---
> > kernel/dma/contiguous.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> > index 69cfb4345388..1b689b1303cd 100644
> > --- a/kernel/dma/contiguous.c
> > +++ b/kernel/dma/contiguous.c
> > @@ -42,10 +42,10 @@ struct cma *dma_contiguous_default_area;
> > * Users, who want to set the size of global CMA area for their system
> > * should use cma= kernel parameter.
> > */
> > -static const phys_addr_t size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> > -static phys_addr_t size_cmdline = -1;
> > -static phys_addr_t base_cmdline;
> > -static phys_addr_t limit_cmdline;
> > +static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
>
> The 0day bot reported an issue with this change with clang:
>
> https://groups.google.com/d/msgid/clang-built-linux/201910140334.nhultlt8%25lkp%40intel.com
>
> kernel/dma/contiguous.c:46:36: error: 'size_cmdline' causes a section type conflict with 'size_bytes'
> static phys_addr_t __ro_after_init size_cmdline = -1;
> ^
> kernel/dma/contiguous.c:45:42: note: declared here
> static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> ^
> kernel/dma/contiguous.c:47:36: error: 'base_cmdline' causes a section type conflict with 'size_bytes'
> static phys_addr_t __ro_after_init base_cmdline;
> ^
> kernel/dma/contiguous.c:45:42: note: declared here
> static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> ^
> kernel/dma/contiguous.c:48:36: error: 'limit_cmdline' causes a section type conflict with 'size_bytes'
> static phys_addr_t __ro_after_init limit_cmdline;
> ^
> kernel/dma/contiguous.c:45:42: note: declared here
> static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> ^
> 3 errors generated.
Thanks for your feedback and reporting this error.
> The errors seem kind of cryptic at first but something that is const
> should automatically be in the read only section, this part of the
> commit seems unnecessary. Removing that part of the change fixes the error.
I have overlooked size_bytes variable
It shouldn't be const if it is declared as __ro_after_init.
I will fix and resend it.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel: dma: Make CMA boot parameters __ro_after_init
2019-10-12 12:29 [PATCH] kernel: dma: Make CMA boot parameters __ro_after_init Shyam Saini
2019-10-14 2:25 ` Nathan Chancellor
@ 2019-10-14 14:33 ` Robin Murphy
2019-10-19 5:37 ` Shyam Saini
1 sibling, 1 reply; 5+ messages in thread
From: Robin Murphy @ 2019-10-14 14:33 UTC (permalink / raw)
To: Shyam Saini, kernel-hardening
Cc: Kees Cook, linux-kernel, Matthew Wilcox, linux-mm, iommu,
Christopher Lameter, Christoph Hellwig
On 12/10/2019 13:29, Shyam Saini wrote:
> This parameters are not changed after early boot.
> By making them __ro_after_init will reduce any attack surface in the
> kernel.
At a glance, it looks like these are only referenced by a couple of
__init functions, so couldn't they just be __initdata/__initconst?
Robin.
> Link: https://lwn.net/Articles/676145/
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Christopher Lameter <cl@linux.com>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Shyam Saini <mayhs11saini@gmail.com>
> ---
> kernel/dma/contiguous.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 69cfb4345388..1b689b1303cd 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -42,10 +42,10 @@ struct cma *dma_contiguous_default_area;
> * Users, who want to set the size of global CMA area for their system
> * should use cma= kernel parameter.
> */
> -static const phys_addr_t size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> -static phys_addr_t size_cmdline = -1;
> -static phys_addr_t base_cmdline;
> -static phys_addr_t limit_cmdline;
> +static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> +static phys_addr_t __ro_after_init size_cmdline = -1;
> +static phys_addr_t __ro_after_init base_cmdline;
> +static phys_addr_t __ro_after_init limit_cmdline;
>
> static int __init early_cma(char *p)
> {
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] kernel: dma: Make CMA boot parameters __ro_after_init
2019-10-14 14:33 ` Robin Murphy
@ 2019-10-19 5:37 ` Shyam Saini
0 siblings, 0 replies; 5+ messages in thread
From: Shyam Saini @ 2019-10-19 5:37 UTC (permalink / raw)
To: Robin Murphy
Cc: Kernel Hardening, Kees Cook, linux-kernel, Matthew Wilcox,
linux-mm, iommu, Christopher Lameter, Christoph Hellwig
Hi Robin,
Sorry for the late reply.
> This parameters are not changed after early boot.
> > By making them __ro_after_init will reduce any attack surface in the
> > kernel.
>
> At a glance, it looks like these are only referenced by a couple of
> __init functions, so couldn't they just be __initdata/__initconst?
yes, You are right it is only used by __init calls and not used anywhere else.
I will resend the updated version.
Thanks a lot for the feedback.
> > Link: https://lwn.net/Articles/676145/
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Cc: Matthew Wilcox <willy@infradead.org>
> > Cc: Christopher Lameter <cl@linux.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Shyam Saini <mayhs11saini@gmail.com>
> > ---
> > kernel/dma/contiguous.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> > index 69cfb4345388..1b689b1303cd 100644
> > --- a/kernel/dma/contiguous.c
> > +++ b/kernel/dma/contiguous.c
> > @@ -42,10 +42,10 @@ struct cma *dma_contiguous_default_area;
> > * Users, who want to set the size of global CMA area for their system
> > * should use cma= kernel parameter.
> > */
> > -static const phys_addr_t size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> > -static phys_addr_t size_cmdline = -1;
> > -static phys_addr_t base_cmdline;
> > -static phys_addr_t limit_cmdline;
> > +static const phys_addr_t __ro_after_init size_bytes = (phys_addr_t)CMA_SIZE_MBYTES * SZ_1M;
> > +static phys_addr_t __ro_after_init size_cmdline = -1;
> > +static phys_addr_t __ro_after_init base_cmdline;
> > +static phys_addr_t __ro_after_init limit_cmdline;
> >
> > static int __init early_cma(char *p)
> > {
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-19 5:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-12 12:29 [PATCH] kernel: dma: Make CMA boot parameters __ro_after_init Shyam Saini
2019-10-14 2:25 ` Nathan Chancellor
2019-10-16 10:11 ` Shyam Saini
2019-10-14 14:33 ` Robin Murphy
2019-10-19 5:37 ` Shyam Saini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).