On Nov 6, 2025 Ricardo Robaina <[email protected]> wrote:
>
> Netfilter code (net/netfilter/nft_log.c and net/netfilter/xt_AUDIT.c)
> have to be kept in sync. Both source files had duplicated versions of
> audit_ip4() and audit_ip6() functions, which can result in lack of
> consistency and/or duplicated work.
>
> This patch adds two helper functions in audit.c that can be called by
> netfilter code commonly, aiming to improve maintainability and
> consistency.
>
> Signed-off-by: Ricardo Robaina <[email protected]>
> Acked-by: Florian Westphal <[email protected]>
> ---
> include/linux/audit.h | 12 +++++++++++
> kernel/audit.c | 39 ++++++++++++++++++++++++++++++++++++
> net/netfilter/nft_log.c | 43 ++++------------------------------------
> net/netfilter/xt_AUDIT.c | 43 ++++------------------------------------
> 4 files changed, 59 insertions(+), 78 deletions(-)
...
> diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
> index e35588137995..f53fb4222134 100644
> --- a/net/netfilter/nft_log.c
> +++ b/net/netfilter/nft_log.c
> @@ -26,41 +26,6 @@ struct nft_log {
> char *prefix;
> };
>
> -static bool audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
> -{
> - struct iphdr _iph;
> - const struct iphdr *ih;
> -
> - ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph),
> &_iph);
> - if (!ih)
> - return false;
> -
> - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
> - &ih->saddr, &ih->daddr, ih->protocol);
> -
> - return true;
> -}
> -
> -static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
> -{
> - struct ipv6hdr _ip6h;
> - const struct ipv6hdr *ih;
> - u8 nexthdr;
> - __be16 frag_off;
> -
> - ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h),
> &_ip6h);
> - if (!ih)
> - return false;
> -
> - nexthdr = ih->nexthdr;
> - ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h),
> &nexthdr, &frag_off);
> -
> - audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu",
> - &ih->saddr, &ih->daddr, nexthdr);
> -
> - return true;
> -}
> -
> static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
> {
> struct sk_buff *skb = pkt->skb;
> @@ -80,18 +45,18 @@ static void nft_log_eval_audit(const struct nft_pktinfo
> *pkt)
> case NFPROTO_BRIDGE:
> switch (eth_hdr(skb)->h_proto) {
> case htons(ETH_P_IP):
> - fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> + fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> break;
> case htons(ETH_P_IPV6):
> - fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> + fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> break;
> }
> break;
> case NFPROTO_IPV4:
> - fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> + fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> break;
> case NFPROTO_IPV6:
> - fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> + fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> break;
> }
We can probably take this a step further by moving the case statements
into the audit functions too. I think this will make some of the other
changes a bit cleaner and should reduce the amount of audit code in the
NFT code.
If we don't want to do that, it might be worthwhile to take the
NFPROTO_BRIDGE protocol family reset shown below in audit_log_nft_skb()
and use that in the nft_log_eval_audit() function so we aren't
duplicating calls into the audit code.
[WARNING: completely untested code, but you should get the basic idea]
diff --git a/kernel/audit.c b/kernel/audit.c
index 26a332ffb1b8..72ba3f51f859 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2538,6 +2538,59 @@ static void audit_log_set_loginuid(kuid_t koldloginuid,
kuid_t kloginuid,
audit_log_end(ab);
}
+int audit_log_nft_skb(struct audit_buffer *ab,
+ struct sk_buff *skb, u8 nfproto)
+{
+ /* find the IP protocol in the case of NFPROTO_BRIDGE */
+ if (nfproto == NFPROTO_BRIDGE) {
+ switch (eth_hdr(skb)->h_proto) {
+ case htons(ETH_P_IP):
+ nfproto = NFPROTO_IPV4;
+ case htons(ETH_P_IPV6):
+ nfproto = NFPROTO_IPV6;
+ default:
+ goto unknown_proto;
+ }
+ }
+
+ switch (nfproto) {
+ case NFPROTO_IPV4: {
+ struct iphdr iph;
+ const struct iphdr *ih;
+
+ ih = skb_header_pointer(skb, skb_network_offset(skb),
+ sizeof(_iph), &_iph);
+ if (!ih)
+ return -ENOMEM;
+
+ audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
+ &ih->saddr, &ih->daddr, ih->protocol);
+ break;
+ }
+ case NFPROTO_IPV6: {
+ struct ipv6hdr iph;
+ const struct ipv6hdr *ih;
+
+ ih = skb_header_pointer(skb, skb_network_offset(skb),
+ sizeof(_iph), &_iph);
+ if (!ih)
+ return -ENOMEM;
+
+ audit_log_format(ab, " saddr=%pI6 daddr=%pI6 proto=%hhu",
+ &ih->saddr, &ih->daddr, ih->protocol);
+ break;
+ }
+ default:
+ goto unknown_proto;
+ }
+
+ return 0;
+
+unknown_proto:
+ audit_log_format(ab, " saddr=? daddr=? proto=?");
+ return -EPFNOSUPPORT;
+}
+
/**
* audit_set_loginuid - set current task's loginuid
* @loginuid: loginuid value
diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
index e35588137995..6f444e2ad70a 100644
--- a/net/netfilter/nft_log.c
+++ b/net/netfilter/nft_log.c
@@ -75,28 +75,7 @@ static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
return;
audit_log_format(ab, "mark=%#x", skb->mark);
-
- switch (nft_pf(pkt)) {
- case NFPROTO_BRIDGE:
- switch (eth_hdr(skb)->h_proto) {
- case htons(ETH_P_IP):
- fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
- break;
- case htons(ETH_P_IPV6):
- fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
- break;
- }
- break;
- case NFPROTO_IPV4:
- fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
- break;
- case NFPROTO_IPV6:
- fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
- break;
- }
-
- if (fam == -1)
- audit_log_format(ab, " saddr=? daddr=? proto=-1");
+ audit_log_nft_skb(ab, skb, nft_pf(pkt));
audit_log_end(ab);
}
--
paul-moore.com
