When CONFIG_AUDIT is set, its CONFIG_NET dependency is also set, and the
dev_get_by_index and init_net symbols (used by dump_common_audit_data)
are found by the linker.  dump_common_audit_data() should then failed to
build when CONFIG_NET is not set. However, because the compiler is
smart, it knows that audit_log_start() always return NULL when
!CONFIG_AUDIT, and it doesn't build the body of common_lsm_audit().  As
a side effect, dump_common_audit_data() is not built and the linker
doesn't error out because of missing symbols.

Let's only build lsm_audit.o when CONFIG_AUDIT is set.

ipv4_skb_to_auditdata() and ipv6_skb_to_auditdata() are only used by
Smack if CONFIG_AUDIT is set, so they don't need fake implementations.

Because common_lsm_audit() is used in multiple places without
CONFIG_AUDIT checks, add a fake implementation.

Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: James Morris <jmorris@namei.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241022161009.982584-2-mic@digikod.net
---
 include/linux/lsm_audit.h | 14 ++++++++++++++
 security/Makefile         |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 97a8b21eb033..c2b01380262c 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -116,14 +116,28 @@ struct common_audit_data {
 #define v4info fam.v4
 #define v6info fam.v6
 
+#ifdef CONFIG_AUDIT
+
 int ipv4_skb_to_auditdata(struct sk_buff *skb,
 		struct common_audit_data *ad, u8 *proto);
 
+#if IS_ENABLED(CONFIG_IPV6)
 int ipv6_skb_to_auditdata(struct sk_buff *skb,
 		struct common_audit_data *ad, u8 *proto);
+#endif /* IS_ENABLED(CONFIG_IPV6) */
 
 void common_lsm_audit(struct common_audit_data *a,
 	void (*pre_audit)(struct audit_buffer *, void *),
 	void (*post_audit)(struct audit_buffer *, void *));
 
+#else /* CONFIG_AUDIT */
+
+static inline void common_lsm_audit(struct common_audit_data *a,
+	void (*pre_audit)(struct audit_buffer *, void *),
+	void (*post_audit)(struct audit_buffer *, void *))
+{
+}
+
+#endif /* CONFIG_AUDIT */
+
 #endif
diff --git a/security/Makefile b/security/Makefile
index cc0982214b84..e25da79f55d3 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -15,7 +15,7 @@ obj-$(CONFIG_SECURITY)			+= security.o
 obj-$(CONFIG_SECURITYFS)		+= inode.o
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/
 obj-$(CONFIG_SECURITY_SMACK)		+= smack/
-obj-$(CONFIG_SECURITY)			+= lsm_audit.o
+obj-$(CONFIG_AUDIT)			+= lsm_audit.o
 obj-$(CONFIG_SECURITY_TOMOYO)		+= tomoyo/
 obj-$(CONFIG_SECURITY_APPARMOR)		+= apparmor/
 obj-$(CONFIG_SECURITY_YAMA)		+= yama/
-- 
2.47.0

On Tue, Oct 22, 2024 at 06:09:56PM +0200, Mickaël Salaün wrote:
> When CONFIG_AUDIT is set, its CONFIG_NET dependency is also set, and the
> dev_get_by_index and init_net symbols (used by dump_common_audit_data)
> are found by the linker.  dump_common_audit_data() should then failed to
> build when CONFIG_NET is not set. However, because the compiler is
> smart, it knows that audit_log_start() always return NULL when
> !CONFIG_AUDIT, and it doesn't build the body of common_lsm_audit().  As
> a side effect, dump_common_audit_data() is not built and the linker
> doesn't error out because of missing symbols.
> 
> Let's only build lsm_audit.o when CONFIG_AUDIT is set.
> 

CONFIG_AUDIT and CONFIG_SECURITY are independent of each other.
With CONFIG_SECURITY=n and CONFIG_AUDIT=y, we now get:

Error log:
arm-linux-gnueabi-ld: security/lsm_audit.o: in function `audit_log_lsm_data':
security/lsm_audit.c:417:(.text+0x5e4): undefined reference to `lockdown_reasons'
arm-linux-gnueabi-ld: security/lsm_audit.c:417:(.text+0x5e8): undefined reference to `lockdown_reasons'
make[3]: *** [scripts/Makefile.vmlinux:78: vmlinux] Error 1
make[2]: *** [Makefile:1178: vmlinux] Error 2
make[1]: *** [Makefile:224: __sub-make] Error 2
make: *** [Makefile:224: __sub-make] Error 2

Guenter

On Wed, Oct 23, 2024 at 2:51 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, Oct 22, 2024 at 06:09:56PM +0200, Mickaël Salaün wrote:
> > When CONFIG_AUDIT is set, its CONFIG_NET dependency is also set, and the
> > dev_get_by_index and init_net symbols (used by dump_common_audit_data)
> > are found by the linker.  dump_common_audit_data() should then failed to
> > build when CONFIG_NET is not set. However, because the compiler is
> > smart, it knows that audit_log_start() always return NULL when
> > !CONFIG_AUDIT, and it doesn't build the body of common_lsm_audit().  As
> > a side effect, dump_common_audit_data() is not built and the linker
> > doesn't error out because of missing symbols.
> >
> > Let's only build lsm_audit.o when CONFIG_AUDIT is set.
>
> CONFIG_AUDIT and CONFIG_SECURITY are independent of each other.
> With CONFIG_SECURITY=n and CONFIG_AUDIT=y, we now get:

Yes, unfortunately the error was seen during linux-next testing too.
I'm removing patch 1/14 from lsm/dev now.

> Error log:
> arm-linux-gnueabi-ld: security/lsm_audit.o: in function `audit_log_lsm_data':
> security/lsm_audit.c:417:(.text+0x5e4): undefined reference to `lockdown_reasons'
> arm-linux-gnueabi-ld: security/lsm_audit.c:417:(.text+0x5e8): undefined reference to `lockdown_reasons'
> make[3]: *** [scripts/Makefile.vmlinux:78: vmlinux] Error 1
> make[2]: *** [Makefile:1178: vmlinux] Error 2
> make[1]: *** [Makefile:224: __sub-make] Error 2
> make: *** [Makefile:224: __sub-make] Error 2

-- 
paul-moore.com

On Tue, Oct 22, 2024 at 12:10 PM Mickaël Salaün <mic@digikod.net> wrote:
>
> When CONFIG_AUDIT is set, its CONFIG_NET dependency is also set, and the
> dev_get_by_index and init_net symbols (used by dump_common_audit_data)
> are found by the linker.  dump_common_audit_data() should then failed to
> build when CONFIG_NET is not set. However, because the compiler is
> smart, it knows that audit_log_start() always return NULL when
> !CONFIG_AUDIT, and it doesn't build the body of common_lsm_audit().  As
> a side effect, dump_common_audit_data() is not built and the linker
> doesn't error out because of missing symbols.
>
> Let's only build lsm_audit.o when CONFIG_AUDIT is set.
>
> ipv4_skb_to_auditdata() and ipv6_skb_to_auditdata() are only used by
> Smack if CONFIG_AUDIT is set, so they don't need fake implementations.
>
> Because common_lsm_audit() is used in multiple places without
> CONFIG_AUDIT checks, add a fake implementation.
>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Serge E. Hallyn <serge@hallyn.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241022161009.982584-2-mic@digikod.net
> ---
>  include/linux/lsm_audit.h | 14 ++++++++++++++
>  security/Makefile         |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)

I think this fix is the right thing to do regardless of the rest of
the patchset so I've merged it into lsm/dev, if anyone objects please
speak up.

Thanks!

-- 
paul-moore.com