| Message ID | 20240826120449.1666461-8-yukaixiong@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | sysctl: move sysctls from vm_table into its own files | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
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
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
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.
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.
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;
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(-)