[PATCH ipsec] xfrm: account XFRMA_MARK in xfrm_sa_len()

[PATCH ipsec] xfrm: account XFRMA_MARK in xfrm_sa_len()

[Yasuaki Torimaru]

xfrm_notify_sa() sizes its netlink message with xfrm_sa_len(), while
copy_to_user_state_extra() appends XFRMA_MARK whenever the state carries
a mark.  xfrm_sa_len() does not count that attribute.  Only the
XFRM_MSG_DELSA path adds it to the length separately, so for a marked
state the XFRM_MSG_NEWSA and XFRM_MSG_UPDSA messages are sized short by
nla_total_size(sizeof(struct xfrm_mark)), 12 bytes.  The other length
calculations in this file already account for the mark; xfrm_sa_len() is
the only one that does not.

Instrumenting xfrm_notify_sa() on an x86-64 KASAN build under qemu to
print the accounted length while adding a marked state confirms the gap:
accounting the mark raises the computed length by exactly 12 bytes
(348 -> 360), matching nla_total_size(sizeof(struct xfrm_mark)).

Whether the shortfall is reached depends on the skb allocation, which is
rounded up: xfrm_notify_sa() fails with -EMSGSIZE only when that slack is
exhausted.  km_state_notify() discards the return value, so such a
failure would be silent.  This was not reproduced.

Count the mark in xfrm_sa_len() for every event and drop the now
redundant addition in the DELSA path.  The message content is unchanged;
only the computed length changes.

Fixes: 6f26b61e177e ("xfrm: Allow user space config of SAD mark")
Signed-off-by: Yasuaki Torimaru <yasuakitorimaru@gmail.com>
---
 net/xfrm/xfrm_user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index a2587c7e796b..5c6ee35bc012 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -4046,6 +4046,8 @@ static inline unsigned int xfrm_sa_len(struct xfrm_state *x)
 		l += nla_total_size(sizeof(*x->encap));
 	if (x->tfcpad)
 		l += nla_total_size(sizeof(x->tfcpad));
+	if (x->mark.v | x->mark.m)
+		l += nla_total_size(sizeof(struct xfrm_mark));
 	if (x->replay_esn)
 		l += nla_total_size(xfrm_replay_state_esn_len(x->replay_esn));
 	else
@@ -4101,7 +4103,6 @@ static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
 	if (c->event == XFRM_MSG_DELSA) {
 		len += nla_total_size(headlen);
 		headlen = sizeof(*id);
-		len += nla_total_size(sizeof(struct xfrm_mark));
 	}
 	len += NLMSG_ALIGN(headlen);
 
-- 
2.54.0

Re: [PATCH ipsec] xfrm: account XFRMA_MARK in xfrm_sa_len()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.