Messages in this thread | | | From | Kees Cook <> | Date | Wed, 6 Dec 2017 15:26:22 -0800 | Subject | Re: [kernel-hardening][PATCH v3 1/3] arm: mm: dump: make page table dumping reusable |
| |
On Wed, Dec 6, 2017 at 1:45 AM, Jinbum Park <jinb.park7@gmail.com> wrote: >>> +#ifndef __ASM_PTDUMP_H >>> +#define __ASM_PTDUMP_H >>> + >>> +#ifdef CONFIG_ARM_PTDUMP_CORE >> >> Is this #ifdef needed? I think this file is only included in dump.c >> and ptdump_debugfs.c, both of which are only built when >> CONFIG_ARM_PTDUMP_CORE is defined. > > Looking at next patch in this patch-set series ([PATCH v3 3/3] arm: > mm: dump: add checking for writable and executable pages), > Not only dump.c and ptdump_debugfs.c but also arch/arm/mm/init.c > include this file (ptdump.h) to call debug_checkwx(). > mm/init.c is not built only when CONFIG_ARM_PTDUMP_CORE is defined. > So, This #ifdef seems not be needed for this patch, but is needed for > this patch-set series. > > >>> +static int ptdump_init(void) >>> +{ >>> + ptdump_initialize(); >>> + return ptdump_debugfs_register(&kernel_ptdump_info, >>> + "kernel_page_tables"); >> >> This changes the return value of ptdump_init. This should do similar >> to what was done before: >> >> return ptdump_debugfs_register(&kernel_ptdump_info, >> "kernel_page_tables") ? 0 : -ENOMEM; > > > ptdump_debugfs_register() already returns what you think. > >>> +int ptdump_debugfs_register(struct ptdump_info *info, const char *name) >>> +{ >>> + struct dentry *pe; >>> + >>> + pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops); >>> + return pe ? 0 : -ENOMEM; >>> + >>> +} > > So "return ptdump_debugfs_register(~~)" is fine.
Ah! Yes, I totally missed the change from create_file to debugfs_register. Sorry for the noise!
-Kees
-- Kees Cook Pixel Security
| |