Petr Mladek <pmla...@suse.com> writes:
> The commit 66cc69e34e86a231 ("Fix: module signature vs tracepoints:
> add new TAINT_UNSIGNED_MODULE") updated module_taint_flags() to
> potentially print one more character. But it did not increase the
> size of the corresponding buffers in m_show() and print_modules().I agree, nice work! Minor nitpick: the winged ' /* 0 */' comments imply the values matter, but they don't. I'd skip that. I've CC'd Jessica to add to her review pile :) Thanks, Rusty. > We have recently done the same mistake when adding a taint flag > for livepatching, see > https://lkml.kernel.org/g/cfba2c823bb984690b73572aaae1db596b54a082.1472137475.git.jpoim...@redhat.com > > Let's convert the taint flags into enum and handle the buffer size > almost automatically. > > It is not optimal because only few taint flags can be printed by > module_taint_flags(). But better be on the safe side. IMHO, it is > not worth the optimization and this is a good compromise. > > Signed-off-by: Petr Mladek <pmla...@suse.com> > --- > include/linux/kernel.h | 44 ++++++++++++++++++++++++-------------------- > kernel/module.c | 8 ++++++-- > kernel/panic.c | 4 ++-- > 3 files changed, 32 insertions(+), 24 deletions(-) > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index d96a6118d26a..1809bc82b7a5 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -472,14 +472,10 @@ static inline void set_arch_panic_timeout(int timeout, > int arch_default_timeout) > if (panic_timeout == arch_default_timeout) > panic_timeout = timeout; > } > -extern const char *print_tainted(void); > enum lockdep_ok { > LOCKDEP_STILL_OK, > LOCKDEP_NOW_UNRELIABLE > }; > -extern void add_taint(unsigned flag, enum lockdep_ok); > -extern int test_taint(unsigned flag); > -extern unsigned long get_taint(void); > extern int root_mountflags; > > extern bool early_boot_irqs_disabled; > @@ -493,22 +489,30 @@ extern enum system_states { > SYSTEM_RESTART, > } system_state; > > -#define TAINT_PROPRIETARY_MODULE 0 > -#define TAINT_FORCED_MODULE 1 > -#define TAINT_CPU_OUT_OF_SPEC 2 > -#define TAINT_FORCED_RMMOD 3 > -#define TAINT_MACHINE_CHECK 4 > -#define TAINT_BAD_PAGE 5 > -#define TAINT_USER 6 > -#define TAINT_DIE 7 > -#define TAINT_OVERRIDDEN_ACPI_TABLE 8 > -#define TAINT_WARN 9 > -#define TAINT_CRAP 10 > -#define TAINT_FIRMWARE_WORKAROUND 11 > -#define TAINT_OOT_MODULE 12 > -#define TAINT_UNSIGNED_MODULE 13 > -#define TAINT_SOFTLOCKUP 14 > -#define TAINT_LIVEPATCH 15 > +enum taint_flags { > + TAINT_PROPRIETARY_MODULE, /* 0 */ > + TAINT_FORCED_MODULE, /* 1 */ > + TAINT_CPU_OUT_OF_SPEC, /* 2 */ > + TAINT_FORCED_RMMOD, /* 3 */ > + TAINT_MACHINE_CHECK, /* 4 */ > + TAINT_BAD_PAGE, /* 5 */ > + TAINT_USER, /* 6 */ > + TAINT_DIE, /* 7 */ > + TAINT_OVERRIDDEN_ACPI_TABLE, /* 8 */ > + TAINT_WARN, /* 9 */ > + TAINT_CRAP, /* 10 */ > + TAINT_FIRMWARE_WORKAROUND, /* 11 */ > + TAINT_OOT_MODULE, /* 12 */ > + TAINT_UNSIGNED_MODULE, /* 13 */ > + TAINT_SOFTLOCKUP, /* 14 */ > + TAINT_LIVEPATCH, /* 15 */ > + TAINT_FLAGS_COUNT /* keep last! */ > +}; > + > +extern const char *print_tainted(void); > +extern void add_taint(enum taint_flags flag, enum lockdep_ok); > +extern int test_taint(enum taint_flags flag); > +extern unsigned long get_taint(void); > > extern const char hex_asc[]; > #define hex_asc_lo(x) hex_asc[((x) & 0x0f)] > diff --git a/kernel/module.c b/kernel/module.c > index 529efae9f481..fb6c0d425b47 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -4036,6 +4036,10 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, > const char *, > } > #endif /* CONFIG_KALLSYMS */ > > +/* Maximum number of characters written by module_flags() */ > +#define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4) > + > +/* Keep in sync with MODULE_FLAGS_BUF_SIZE !!! */ > static char *module_flags(struct module *mod, char *buf) > { > int bx = 0; > @@ -4080,7 +4084,7 @@ static void m_stop(struct seq_file *m, void *p) > static int m_show(struct seq_file *m, void *p) > { > struct module *mod = list_entry(p, struct module, list); > - char buf[8]; > + char buf[MODULE_FLAGS_BUF_SIZE]; > > /* We always ignore unformed modules. */ > if (mod->state == MODULE_STATE_UNFORMED) > @@ -4251,7 +4255,7 @@ EXPORT_SYMBOL_GPL(__module_text_address); > void print_modules(void) > { > struct module *mod; > - char buf[8]; > + char buf[MODULE_FLAGS_BUF_SIZE]; > > printk(KERN_DEFAULT "Modules linked in:"); > /* Most callers should already have preempt disabled, but make sure */ > diff --git a/kernel/panic.c b/kernel/panic.c > index ca8cea1ef673..e90125bf9238 100644 > --- a/kernel/panic.c > +++ b/kernel/panic.c > @@ -334,7 +334,7 @@ const char *print_tainted(void) > return buf; > } > > -int test_taint(unsigned flag) > +int test_taint(enum taint_flags flag) > { > return test_bit(flag, &tainted_mask); > } > @@ -353,7 +353,7 @@ unsigned long get_taint(void) > * If something bad has gone wrong, you'll want @lockdebug_ok = false, but > for > * some notewortht-but-not-corrupting cases, it can be set to true. > */ > -void add_taint(unsigned flag, enum lockdep_ok lockdep_ok) > +void add_taint(enum taint_flags flag, enum lockdep_ok lockdep_ok) > { > if (lockdep_ok == LOCKDEP_NOW_UNRELIABLE && __debug_locks_off()) > pr_warn("Disabling lock debugging due to kernel taint\n"); > -- > 1.8.5.6
