Messages in this thread | | | From | Justin Stitt <> | Date | Thu, 31 Aug 2023 16:18:10 -0700 | Subject | Re: [PATCH] x86/audit: fix -Wmissing-variable-declarations warning for ia32_xyz_class |
| |
On Thu, Aug 31, 2023 at 2:11 PM Kees Cook <keescook@chromium.org> wrote: > > On Tue, Aug 29, 2023 at 10:33:16PM +0000, Justin Stitt wrote: > > When building x86 defconfig with Clang-18 I get the following warnings: > > | arch/x86/ia32/audit.c:6:10: warning: no previous extern declaration for non-static variable 'ia32_dir_class' [-Wmissing-variable-declarations] > > | 6 | unsigned ia32_dir_class[] = { > > | arch/x86/ia32/audit.c:11:10: warning: no previous extern declaration for non-static variable 'ia32_chattr_class' [-Wmissing-variable-declarations] > > | 11 | unsigned ia32_chattr_class[] = { > > | arch/x86/ia32/audit.c:16:10: warning: no previous extern declaration for non-static variable 'ia32_write_class' [-Wmissing-variable-declarations] > > | 16 | unsigned ia32_write_class[] = { > > | arch/x86/ia32/audit.c:21:10: warning: no previous extern declaration for non-static variable 'ia32_read_class' [-Wmissing-variable-declarations] > > | 21 | unsigned ia32_read_class[] = { > > | arch/x86/ia32/audit.c:26:10: warning: no previous extern declaration for non-static variable 'ia32_signal_class' [-Wmissing-variable-declarations] > > | 26 | unsigned ia32_signal_class[] = { > > > > These warnings occur due to their respective extern declarations being > > scoped inside of audit_classes_init as well as only being enabled with > > `CONFIG_IA32_EMULATION=y`: > > | static int __init audit_classes_init(void) > > | { > > | #ifdef CONFIG_IA32_EMULATION > > | extern __u32 ia32_dir_class[]; > > | extern __u32 ia32_write_class[]; > > | extern __u32 ia32_read_class[]; > > | extern __u32 ia32_chattr_class[]; > > | audit_register_class(AUDIT_CLASS_WRITE_32, ia32_write_class); > > | audit_register_class(AUDIT_CLASS_READ_32, ia32_read_class); > > | audit_register_class(AUDIT_CLASS_DIR_WRITE_32, ia32_dir_class); > > | audit_register_class(AUDIT_CLASS_CHATTR_32, ia32_chattr_class); > > | #endif > > | audit_register_class(AUDIT_CLASS_WRITE, write_class); > > | audit_register_class(AUDIT_CLASS_READ, read_class); > > | audit_register_class(AUDIT_CLASS_DIR_WRITE, dir_class); > > | audit_register_class(AUDIT_CLASS_CHATTR, chattr_class); > > | return 0; > > | } > > > > Lift the extern declarations to their own header and resolve scoping > > issues (and thus fix the warnings). > > > > Moreover, change __u32 to unsigned so that we match the definitions: > > | unsigned ia32_dir_class[] = { > > | #include <asm-generic/audit_dir_write.h> > > | ~0U > > | }; > > | > > | unsigned ia32_chattr_class[] = { > > | #include <asm-generic/audit_change_attr.h> > > | ~0U > > | }; > > | ... > > I would expect checkpatch to warn about bare "unsigned", which is frown > on these days. :) I think __u32 should be fine here...? (Why is it __u32 > instead of u32, btw?)
Yeah, checkpatch doesn't like it. I was just trying to mirror the implementation in audit.c as closely as possible: | unsigned ia32_dir_class[] = { | #include <asm-generic/audit_dir_write.h> | ~0U | };
> > But otherwise, yes, looks good.
Thanks for the feedback here. Should I send a v2 where I changed _all_ instances of `unsigned` in both audit.c and audit.h to be `u32`? Or perhaps, `unsigned int`.
> > -Kees > > -- > Kees Cook
| |