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

On 2024/8/27 9:56, Paul Moore wrote:
> On Mon, Aug 26, 2024 at 9:38 PM yukaixiong <[email protected]> wrote:
>> On 2024/8/27 6:49, Paul Moore wrote:
>>> On Mon, Aug 26, 2024 at 8:05 AM Kaixiong Yu <[email protected]> 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 <[email protected]>
>>>> ---
>>>>    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.