For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.
***
Subject: [PATCH] xfrm: fix memory leak and improper cleanup in xfrm_add_policy()
Author: kartikey406@gmail.com
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
When xfrm_policy_insert() fails in xfrm_add_policy(), the error
handling path manually calls xfrm_dev_policy_free(), followed by
security_xfrm_policy_free() and a raw kfree(). This is problematic
for three reasons:
1. xfrm_policy_destroy() already calls xfrm_dev_policy_free()
internally, so the manual call results in a double-free.
2. xfrm_policy_destroy() frees the policy asynchronously via
call_rcu(), which schedules xfrm_policy_destroy_rcu() to call
security_xfrm_policy_free() and kfree(). The manual
security_xfrm_policy_free() call before kfree() races with this
RCU callback, causing another double-free.
3. The raw kfree() is not RCU-safe and may free memory that is still
being accessed by concurrent RCU readers.
Fix this by using xfrm_policy_destroy() as the single, correct
cleanup path, consistent with how xfrm_policy_construct() handles
its own error paths. The walk.dead flag must be set first as
xfrm_policy_destroy() requires it via BUG_ON(!policy->walk.dead).
Reported-by: syzbot+901d48e0b95aed4a2548@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=901d48e0b95aed4a2548
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
net/xfrm/xfrm_user.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index d56450f61669..ae144d1e4a65 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2267,9 +2267,8 @@ static int xfrm_add_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err) {
xfrm_dev_policy_delete(xp);
- xfrm_dev_policy_free(xp);
- security_xfrm_policy_free(xp->security);
- kfree(xp);
+ xp->walk.dead = 1;
+ xfrm_policy_destroy(xp);
return err;
}
--
2.43.0