[PATCH net] xfrm: account XFRMA_IF_ID in aevent size calculation

Keenan Dong posted 1 patch 1 week ago
net/xfrm/xfrm_user.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH net] xfrm: account XFRMA_IF_ID in aevent size calculation
Posted by Keenan Dong 1 week ago
xfrm_get_ae() allocates the reply skb with xfrm_aevent_msgsize(), then
build_aevent() appends attributes including XFRMA_IF_ID when x->if_id is
set.

xfrm_aevent_msgsize() does not include space for XFRMA_IF_ID. For states
with if_id, build_aevent() can fail with -EMSGSIZE and hit BUG_ON(err < 0)
in xfrm_get_ae(), turning a malformed netlink interaction into a kernel
panic.

Account XFRMA_IF_ID in the size calculation unconditionally and replace
the BUG_ON with normal error unwinding.

Fixes: 7e6526404ade ("xfrm: Add a new lookup key to match xfrm interfaces.")
Reported-by: Keenan Dong <keenanat2000@gmail.com>
Signed-off-by: Keenan Dong <keenanat2000@gmail.com>
---
 net/xfrm/xfrm_user.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 1656b487f..d79240a1c 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2677,7 +2677,8 @@ static inline unsigned int xfrm_aevent_msgsize(struct xfrm_state *x)
 	       + nla_total_size(4) /* XFRM_AE_RTHR */
 	       + nla_total_size(4) /* XFRM_AE_ETHR */
 	       + nla_total_size(sizeof(x->dir)) /* XFRMA_SA_DIR */
-	       + nla_total_size(4); /* XFRMA_SA_PCPU */
+	       + nla_total_size(4) /* XFRMA_SA_PCPU */
+	       + nla_total_size(sizeof(x->if_id)); /* XFRMA_IF_ID */
 }
 
 static int build_aevent(struct sk_buff *skb, struct xfrm_state *x, const struct km_event *c)
@@ -2789,7 +2790,12 @@ static int xfrm_get_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
 	c.portid = nlh->nlmsg_pid;
 
 	err = build_aevent(r_skb, x, &c);
-	BUG_ON(err < 0);
+	if (err < 0) {
+		spin_unlock_bh(&x->lock);
+		xfrm_state_put(x);
+		kfree_skb(r_skb);
+		return err;
+	}
 
 	err = nlmsg_unicast(xfrm_net_nlsk(net, skb), r_skb, NETLINK_CB(skb).portid);
 	spin_unlock_bh(&x->lock);
-- 
2.43.0
Re: [PATCH net] xfrm: account XFRMA_IF_ID in aevent size calculation
Posted by Steffen Klassert 2 days, 11 hours ago
On Thu, Mar 26, 2026 at 08:36:39PM +0800, Keenan Dong wrote:
> xfrm_get_ae() allocates the reply skb with xfrm_aevent_msgsize(), then
> build_aevent() appends attributes including XFRMA_IF_ID when x->if_id is
> set.
> 
> xfrm_aevent_msgsize() does not include space for XFRMA_IF_ID. For states
> with if_id, build_aevent() can fail with -EMSGSIZE and hit BUG_ON(err < 0)
> in xfrm_get_ae(), turning a malformed netlink interaction into a kernel
> panic.
> 
> Account XFRMA_IF_ID in the size calculation unconditionally and replace
> the BUG_ON with normal error unwinding.
> 
> Fixes: 7e6526404ade ("xfrm: Add a new lookup key to match xfrm interfaces.")
> Reported-by: Keenan Dong <keenanat2000@gmail.com>
> Signed-off-by: Keenan Dong <keenanat2000@gmail.com>

Applied to the ipsec tree, thanks a lot!
Re: [PATCH net] xfrm: account XFRMA_IF_ID in aevent size calculation
Posted by Keenan Dong 3 days, 4 hours ago
Hi,

Just a gentle ping on this patch:

I wanted to check if I might have missed any process or
if there’s anything I should revise.

Thanks for your time.

Best regards,
Xianrui Dong


On Thu, Mar 26, 2026 at 8:36 PM Keenan Dong <keenanat2000@gmail.com> wrote:
>
> xfrm_get_ae() allocates the reply skb with xfrm_aevent_msgsize(), then
> build_aevent() appends attributes including XFRMA_IF_ID when x->if_id is
> set.
>
> xfrm_aevent_msgsize() does not include space for XFRMA_IF_ID. For states
> with if_id, build_aevent() can fail with -EMSGSIZE and hit BUG_ON(err < 0)
> in xfrm_get_ae(), turning a malformed netlink interaction into a kernel
> panic.
>
> Account XFRMA_IF_ID in the size calculation unconditionally and replace
> the BUG_ON with normal error unwinding.
>
> Fixes: 7e6526404ade ("xfrm: Add a new lookup key to match xfrm interfaces.")
> Reported-by: Keenan Dong <keenanat2000@gmail.com>
> Signed-off-by: Keenan Dong <keenanat2000@gmail.com>
> ---
>  net/xfrm/xfrm_user.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 1656b487f..d79240a1c 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -2677,7 +2677,8 @@ static inline unsigned int xfrm_aevent_msgsize(struct xfrm_state *x)
>                + nla_total_size(4) /* XFRM_AE_RTHR */
>                + nla_total_size(4) /* XFRM_AE_ETHR */
>                + nla_total_size(sizeof(x->dir)) /* XFRMA_SA_DIR */
> -              + nla_total_size(4); /* XFRMA_SA_PCPU */
> +              + nla_total_size(4) /* XFRMA_SA_PCPU */
> +              + nla_total_size(sizeof(x->if_id)); /* XFRMA_IF_ID */
>  }
>
>  static int build_aevent(struct sk_buff *skb, struct xfrm_state *x, const struct km_event *c)
> @@ -2789,7 +2790,12 @@ static int xfrm_get_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
>         c.portid = nlh->nlmsg_pid;
>
>         err = build_aevent(r_skb, x, &c);
> -       BUG_ON(err < 0);
> +       if (err < 0) {
> +               spin_unlock_bh(&x->lock);
> +               xfrm_state_put(x);
> +               kfree_skb(r_skb);
> +               return err;
> +       }
>
>         err = nlmsg_unicast(xfrm_net_nlsk(net, skb), r_skb, NETLINK_CB(skb).portid);
>         spin_unlock_bh(&x->lock);
> --
> 2.43.0
>