[PATCH nf 4/4] netfilter: nfnetlink_log: rework mac header logging

[Florian Westphal <fw@strlen.de>]

For unknown reasons this uses dev->hard_header_len.

LLM says that hard_header_len says how long a link layer header can get,
not how many bytes this packet holds. Use skb_mac_header_len instead and
cap if needed.

AFAICS this should have dumped skb_mac_header_len bytes instead, with one
single attribute (plus maybe skb->protocol).

Keep it close to the original version: if hard_header_len is larger than
the mac header size of the packet, then clamp to that value.

While at it, also move this so that we don't call this for POST_ROUTING
hook; mac header is still that of the incoming device, not the egress one.

I suspect that this is almost exclusively used with plain ethernet,
where hard_header_len and skb_mac_header_len() are 14 bytes which
would explain absence of bug reports.

Fixes: 72961ecf84d6 ("netfilter: nfnetlink_log: send complete hardware header")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nfnetlink_log.c | 68 +++++++++++++++++++++++++++--------
 1 file changed, 54 insertions(+), 14 deletions(-)

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 29530b753928..e6877ac36d26 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -458,6 +458,56 @@ static int nfulnl_put_bridge(struct nfulnl_instance *inst, const struct sk_buff
 	return -1;
 }
 
+/**
+ * nfulnl_put_hwheader - Append link-layer header info to the netlink skb.
+ * @nlskb:      Target netlink message to append attributes to.
+ * @skb:        Original packet skb whose link-layer header is described.
+ *
+ * Emits the NFULA_HWTYPE and NFULA_HWLEN netlink attributes into @nlskb
+ * based on the link-layer header of @skb.
+ *
+ * The device's hard_header_len describes the maximum size a link-layer
+ * header can grow to, not the actual size of this packet's header.
+ * The actual length is taken from skb_mac_header_len() and capped at
+ * hard_header_len if needed.
+ *
+ * The NFULA_HWLEN attribute is retained for backward compatibility.
+ * Userspace should derive the length from the NFULA_HWHEADER attribute
+ * size instead.
+ *
+ * Caller must check skb_mac_header_was_set(skb) before using this
+ * function.
+ *
+ * Return: -EMSGSIZE if the tailroom of the skb is insufficient to store
+ * the attribute header and payload.
+ */
+static int nfulnl_put_hwheader(struct sk_buff *nlskb, const struct sk_buff *skb)
+{
+	unsigned int hwlen = READ_ONCE(skb->dev->hard_header_len);
+	const unsigned char *hwhdrp = skb_mac_header(skb);
+	int len = skb_mac_header_len(skb);
+	u16 hwtype = skb->dev->type;
+
+	if (nla_put_be16(nlskb, NFULA_HWTYPE, htons(hwtype)))
+		return -EMSGSIZE;
+
+	if (len > hwlen)
+		len = hwlen;
+
+	/* This attribute should not exist, userspace should
+	 * use NFULA_HWHEADER attribute size. Removing
+	 * this breaks existing users which unfortunately
+	 * rely on NFULA_HWLEN presence.
+	 */
+	if (nla_put_be16(nlskb, NFULA_HWLEN, htons(len)))
+		return -EMSGSIZE;
+
+	if (len == 0)
+		return 0;
+
+	return nla_put(nlskb, NFULA_HWHEADER, len, hwhdrp);
+}
+
 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
 static int nflog_put_master_ifindex(struct sk_buff *nlskb, int attr,
 				    const struct net_device *dev)
@@ -494,7 +544,6 @@ __build_packet_message(struct nfnl_log_net *log,
 	struct nlmsghdr *nlh;
 	sk_buff_data_t old_tail = inst->skb->tail;
 	struct sock *sk;
-	const unsigned char *hwhdrp;
 
 	nlh = nfnl_msg_put(inst->skb, 0, 0,
 			   nfnl_msg_type(NFNL_SUBSYS_ULOG, NFULNL_MSG_PACKET),
@@ -592,25 +641,16 @@ __build_packet_message(struct nfnl_log_net *log,
 	if (nfnetlink_put_hwaddr(inst->skb, skb, indev, NFULA_HWADDR))
 		goto nla_put_failure;
 
-	if (indev && skb_mac_header_was_set(skb)) {
-		if (nla_put_be16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type)) ||
-		    nla_put_be16(inst->skb, NFULA_HWLEN,
-				 htons(skb->dev->hard_header_len)))
-			goto nla_put_failure;
-
-		hwhdrp = skb_mac_header(skb);
-
-		if (nla_put(inst->skb, NFULA_HWHEADER,
-			    skb->dev->hard_header_len, hwhdrp))
-			goto nla_put_failure;
-	}
-
 	if (hooknum <= NF_INET_FORWARD) {
 		struct timespec64 kts = ktime_to_timespec64(skb_tstamp_cond(skb, true));
 		struct nfulnl_msg_packet_timestamp ts;
 		ts.sec = cpu_to_be64(kts.tv_sec);
 		ts.usec = cpu_to_be64(kts.tv_nsec / NSEC_PER_USEC);
 
+		if (indev && skb->dev && skb_mac_header_was_set(skb) &&
+		    nfulnl_put_hwheader(inst->skb, skb))
+			goto nla_put_failure;
+
 		if (nla_put(inst->skb, NFULA_TIMESTAMP, sizeof(ts), &ts))
 			goto nla_put_failure;
 	}
-- 
2.56.0