1. Patchew
  2. linux
  3. [PATCH -next 00/15] sysctl: move sysctls from vm_table into its own files
  4. View patch

[PATCH -next 07/15] security: min_addr: move sysctl into its own file

Kaixiong Yu posted 15 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH -next 07/15] security: min_addr: move sysctl into its own file
Posted by Kaixiong Yu 1 year, 3 months ago 
The dac_mmap_min_addr belongs to min_addr.c, move it into
its own file from /kernel/sysctl.c. In the previous Linux kernel
boot process, sysctl_init_bases needs to be executed before
init_mmap_min_addr, So, register_sysctl_init should be executed
before update_mmap_min_addr in init_mmap_min_addr.

Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
---
 kernel/sysctl.c     |  9 ---------
 security/min_addr.c | 11 +++++++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 41d4afc978e6..0c0bab3dad7d 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2059,15 +2059,6 @@ static struct ctl_table vm_table[] = {
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
 	},
-#ifdef CONFIG_MMU
-	{
-		.procname	= "mmap_min_addr",
-		.data		= &dac_mmap_min_addr,
-		.maxlen		= sizeof(unsigned long),
-		.mode		= 0644,
-		.proc_handler	= mmap_min_addr_handler,
-	},
-#endif
 #if (defined(CONFIG_X86_32) && !defined(CONFIG_UML))|| \
    (defined(CONFIG_SUPERH) && defined(CONFIG_VSYSCALL))
 	{
diff --git a/security/min_addr.c b/security/min_addr.c
index 0ce267c041ab..b2f61649e110 100644
--- a/security/min_addr.c
+++ b/security/min_addr.c
@@ -44,8 +44,19 @@ int mmap_min_addr_handler(const struct ctl_table *table, int write,
 	return ret;
 }
 
+static struct ctl_table min_addr_sysctl_table[] = {
+	{
+		.procname	= "mmap_min_addr",
+		.data		= &dac_mmap_min_addr,
+		.maxlen		= sizeof(unsigned long),
+		.mode		= 0644,
+		.proc_handler	= mmap_min_addr_handler,
+	},
+};
+
 static int __init init_mmap_min_addr(void)
 {
+	register_sysctl_init("vm", min_addr_sysctl_table);
 	update_mmap_min_addr();
 
 	return 0;
-- 
2.25.1
Re: [PATCH -next 07/15] security: min_addr: move sysctl into its own file
Posted by Paul Moore 1 year, 3 months ago 
On Mon, Aug 26, 2024 at 8:05 AM Kaixiong Yu <yukaixiong@huawei.com> wrote:
>
> The dac_mmap_min_addr belongs to min_addr.c, move it into
> its own file from /kernel/sysctl.c. In the previous Linux kernel
> boot process, sysctl_init_bases needs to be executed before
> init_mmap_min_addr, So, register_sysctl_init should be executed
> before update_mmap_min_addr in init_mmap_min_addr.
>
> Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
> ---
>  kernel/sysctl.c     |  9 ---------
>  security/min_addr.c | 11 +++++++++++
>  2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 41d4afc978e6..0c0bab3dad7d 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2059,15 +2059,6 @@ static struct ctl_table vm_table[] = {
>                 .proc_handler   = proc_dointvec_minmax,
>                 .extra1         = SYSCTL_ZERO,
>         },
> -#ifdef CONFIG_MMU
> -       {
> -               .procname       = "mmap_min_addr",
> -               .data           = &dac_mmap_min_addr,
> -               .maxlen         = sizeof(unsigned long),
> -               .mode           = 0644,
> -               .proc_handler   = mmap_min_addr_handler,
> -       },
> -#endif
>  #if (defined(CONFIG_X86_32) && !defined(CONFIG_UML))|| \
>     (defined(CONFIG_SUPERH) && defined(CONFIG_VSYSCALL))
>         {
> diff --git a/security/min_addr.c b/security/min_addr.c
> index 0ce267c041ab..b2f61649e110 100644
> --- a/security/min_addr.c
> +++ b/security/min_addr.c
> @@ -44,8 +44,19 @@ int mmap_min_addr_handler(const struct ctl_table *table, int write,
>         return ret;
>  }
>
> +static struct ctl_table min_addr_sysctl_table[] = {
> +       {
> +               .procname       = "mmap_min_addr",
> +               .data           = &dac_mmap_min_addr,
> +               .maxlen         = sizeof(unsigned long),
> +               .mode           = 0644,
> +               .proc_handler   = mmap_min_addr_handler,
> +       },
> +};

I haven't chased all of the Kconfig deps to see if there is a problem,
but please provide a quick explanation in the commit description about
why it is okay to drop the CONFIG_MMU check.

>  static int __init init_mmap_min_addr(void)
>  {
> +       register_sysctl_init("vm", min_addr_sysctl_table);
>         update_mmap_min_addr();
>
>         return 0;
> --
> 2.25.1

-- 
paul-moore.com
Re: [PATCH -next 07/15] security: min_addr: move sysctl into its own file
Posted by yukaixiong 1 year, 3 months ago 

On 2024/8/27 6:49, Paul Moore wrote:
> On Mon, Aug 26, 2024 at 8:05 AM Kaixiong Yu <yukaixiong@huawei.com> wrote:
>> The dac_mmap_min_addr belongs to min_addr.c, move it into
>> its own file from /kernel/sysctl.c. In the previous Linux kernel
>> boot process, sysctl_init_bases needs to be executed before
>> init_mmap_min_addr, So, register_sysctl_init should be executed
>> before update_mmap_min_addr in init_mmap_min_addr.
>>
>> Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
>> ---
>>   kernel/sysctl.c     |  9 ---------
>>   security/min_addr.c | 11 +++++++++++
>>   2 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index 41d4afc978e6..0c0bab3dad7d 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -2059,15 +2059,6 @@ static struct ctl_table vm_table[] = {
>>                  .proc_handler   = proc_dointvec_minmax,
>>                  .extra1         = SYSCTL_ZERO,
>>          },
>> -#ifdef CONFIG_MMU
>> -       {
>> -               .procname       = "mmap_min_addr",
>> -               .data           = &dac_mmap_min_addr,
>> -               .maxlen         = sizeof(unsigned long),
>> -               .mode           = 0644,
>> -               .proc_handler   = mmap_min_addr_handler,
>> -       },
>> -#endif
>>   #if (defined(CONFIG_X86_32) && !defined(CONFIG_UML))|| \
>>      (defined(CONFIG_SUPERH) && defined(CONFIG_VSYSCALL))
>>          {
>> diff --git a/security/min_addr.c b/security/min_addr.c
>> index 0ce267c041ab..b2f61649e110 100644
>> --- a/security/min_addr.c
>> +++ b/security/min_addr.c
>> @@ -44,8 +44,19 @@ int mmap_min_addr_handler(const struct ctl_table *table, int write,
>>          return ret;
>>   }
>>
>> +static struct ctl_table min_addr_sysctl_table[] = {
>> +       {
>> +               .procname       = "mmap_min_addr",
>> +               .data           = &dac_mmap_min_addr,
>> +               .maxlen         = sizeof(unsigned long),
>> +               .mode           = 0644,
>> +               .proc_handler   = mmap_min_addr_handler,
>> +       },
>> +};
> I haven't chased all of the Kconfig deps to see if there is a problem,
> but please provide a quick explanation in the commit description about
> why it is okay to drop the CONFIG_MMU check.

According to the compilation condition in security/Makefile:

               obj-$(CONFIG_MMU)            += min_addr.o

if CONFIG_MMU is not defined, min_addr.c would not be included in the 
compilation process.
So，it is okay to drop the CONFIG_MMU check.
>>   static int __init init_mmap_min_addr(void)
>>   {
>> +       register_sysctl_init("vm", min_addr_sysctl_table);
>>          update_mmap_min_addr();
>>
>>          return 0;
>> --
>> 2.25.1
Re: [PATCH -next 07/15] security: min_addr: move sysctl into its own file
Posted by Paul Moore 1 year, 3 months ago 
On Mon, Aug 26, 2024 at 9:38 PM yukaixiong <yukaixiong@huawei.com> wrote:
> On 2024/8/27 6:49, Paul Moore wrote:
> > On Mon, Aug 26, 2024 at 8:05 AM Kaixiong Yu <yukaixiong@huawei.com> wrote:
> >> The dac_mmap_min_addr belongs to min_addr.c, move it into
> >> its own file from /kernel/sysctl.c. In the previous Linux kernel
> >> boot process, sysctl_init_bases needs to be executed before
> >> init_mmap_min_addr, So, register_sysctl_init should be executed
> >> before update_mmap_min_addr in init_mmap_min_addr.
> >>
> >> Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
> >> ---
> >>   kernel/sysctl.c     |  9 ---------
> >>   security/min_addr.c | 11 +++++++++++
> >>   2 files changed, 11 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> >> index 41d4afc978e6..0c0bab3dad7d 100644
> >> --- a/kernel/sysctl.c
> >> +++ b/kernel/sysctl.c
> >> @@ -2059,15 +2059,6 @@ static struct ctl_table vm_table[] = {
> >>                  .proc_handler   = proc_dointvec_minmax,
> >>                  .extra1         = SYSCTL_ZERO,
> >>          },
> >> -#ifdef CONFIG_MMU
> >> -       {
> >> -               .procname       = "mmap_min_addr",
> >> -               .data           = &dac_mmap_min_addr,
> >> -               .maxlen         = sizeof(unsigned long),
> >> -               .mode           = 0644,
> >> -               .proc_handler   = mmap_min_addr_handler,
> >> -       },
> >> -#endif
> >>   #if (defined(CONFIG_X86_32) && !defined(CONFIG_UML))|| \
> >>      (defined(CONFIG_SUPERH) && defined(CONFIG_VSYSCALL))
> >>          {
> >> diff --git a/security/min_addr.c b/security/min_addr.c
> >> index 0ce267c041ab..b2f61649e110 100644
> >> --- a/security/min_addr.c
> >> +++ b/security/min_addr.c
> >> @@ -44,8 +44,19 @@ int mmap_min_addr_handler(const struct ctl_table *table, int write,
> >>          return ret;
> >>   }
> >>
> >> +static struct ctl_table min_addr_sysctl_table[] = {
> >> +       {
> >> +               .procname       = "mmap_min_addr",
> >> +               .data           = &dac_mmap_min_addr,
> >> +               .maxlen         = sizeof(unsigned long),
> >> +               .mode           = 0644,
> >> +               .proc_handler   = mmap_min_addr_handler,
> >> +       },
> >> +};
> >
> > I haven't chased all of the Kconfig deps to see if there is a problem,
> > but please provide a quick explanation in the commit description about
> > why it is okay to drop the CONFIG_MMU check.
>
> According to the compilation condition in security/Makefile:
>
>                obj-$(CONFIG_MMU)            += min_addr.o
>
> if CONFIG_MMU is not defined, min_addr.c would not be included in the
> compilation process.
> So，it is okay to drop the CONFIG_MMU check.

Great, please add some text about that in the commit description as it
is an important difference in the code changes that isn't currently
documented in the patch.

-- 
paul-moore.com
Re: [PATCH -next 07/15] security: min_addr: move sysctl into its own file
Posted by yukaixiong 1 year, 3 months ago 

On 2024/8/27 9:56, Paul Moore wrote:
> On Mon, Aug 26, 2024 at 9:38 PM yukaixiong <yukaixiong@huawei.com> wrote:
>> On 2024/8/27 6:49, Paul Moore wrote:
>>> On Mon, Aug 26, 2024 at 8:05 AM Kaixiong Yu <yukaixiong@huawei.com> wrote:
>>>> The dac_mmap_min_addr belongs to min_addr.c, move it into
>>>> its own file from /kernel/sysctl.c. In the previous Linux kernel
>>>> boot process, sysctl_init_bases needs to be executed before
>>>> init_mmap_min_addr, So, register_sysctl_init should be executed
>>>> before update_mmap_min_addr in init_mmap_min_addr.
>>>>
>>>> Signed-off-by: Kaixiong Yu <yukaixiong@huawei.com>
>>>> ---
>>>>    kernel/sysctl.c     |  9 ---------
>>>>    security/min_addr.c | 11 +++++++++++
>>>>    2 files changed, 11 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>>>> index 41d4afc978e6..0c0bab3dad7d 100644
>>>> --- a/kernel/sysctl.c
>>>> +++ b/kernel/sysctl.c
>>>> @@ -2059,15 +2059,6 @@ static struct ctl_table vm_table[] = {
>>>>                   .proc_handler   = proc_dointvec_minmax,
>>>>                   .extra1         = SYSCTL_ZERO,
>>>>           },
>>>> -#ifdef CONFIG_MMU
>>>> -       {
>>>> -               .procname       = "mmap_min_addr",
>>>> -               .data           = &dac_mmap_min_addr,
>>>> -               .maxlen         = sizeof(unsigned long),
>>>> -               .mode           = 0644,
>>>> -               .proc_handler   = mmap_min_addr_handler,
>>>> -       },
>>>> -#endif
>>>>    #if (defined(CONFIG_X86_32) && !defined(CONFIG_UML))|| \
>>>>       (defined(CONFIG_SUPERH) && defined(CONFIG_VSYSCALL))
>>>>           {
>>>> diff --git a/security/min_addr.c b/security/min_addr.c
>>>> index 0ce267c041ab..b2f61649e110 100644
>>>> --- a/security/min_addr.c
>>>> +++ b/security/min_addr.c
>>>> @@ -44,8 +44,19 @@ int mmap_min_addr_handler(const struct ctl_table *table, int write,
>>>>           return ret;
>>>>    }
>>>>
>>>> +static struct ctl_table min_addr_sysctl_table[] = {
>>>> +       {
>>>> +               .procname       = "mmap_min_addr",
>>>> +               .data           = &dac_mmap_min_addr,
>>>> +               .maxlen         = sizeof(unsigned long),
>>>> +               .mode           = 0644,
>>>> +               .proc_handler   = mmap_min_addr_handler,
>>>> +       },
>>>> +};
>>> I haven't chased all of the Kconfig deps to see if there is a problem,
>>> but please provide a quick explanation in the commit description about
>>> why it is okay to drop the CONFIG_MMU check.
>> According to the compilation condition in security/Makefile:
>>
>>                 obj-$(CONFIG_MMU)            += min_addr.o
>>
>> if CONFIG_MMU is not defined, min_addr.c would not be included in the
>> compilation process.
>> So，it is okay to drop the CONFIG_MMU check.
> Great, please add some text about that in the commit description as it
> is an important difference in the code changes that isn't currently
> documented in the patch.
ok, I will add the related text in this patch series v2.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.