Commit 3aaea88f authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'atm-clip-fix-infinite-recursion-potential-null-ptr-deref-and-memleak'

Kuniyuki Iwashima says:

====================
atm: clip: Fix infinite recursion, potential null-ptr-deref, and memleak.

Patch 1 fixes racy access to atmarpd found while checking RTNL usage
in clip.c.

Patch 2 fixes memory leak by ioctl(ATMARP_MKIP) and ioctl(ATMARPD_CTRL).

Patch 3 fixes infinite recursive call of clip_vcc->old_push(), which
was reported by syzbot.

v1: https://lore.kernel.org/20250702020437.703698-1-kuniyu@google.com
====================

Link: https://patch.msgid.link/20250704062416.1613927-1-kuniyu@google.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 3c78f91e c489f328
Loading
Loading
Loading
Loading
+39 −15
Original line number Diff line number Diff line
@@ -45,7 +45,8 @@
#include <net/atmclip.h>

static struct net_device *clip_devs;
static struct atm_vcc *atmarpd;
static struct atm_vcc __rcu *atmarpd;
static DEFINE_MUTEX(atmarpd_lock);
static struct timer_list idle_timer;
static const struct neigh_ops clip_neigh_ops;

@@ -53,24 +54,35 @@ static int to_atmarpd(enum atmarp_ctrl_type type, int itf, __be32 ip)
{
	struct sock *sk;
	struct atmarp_ctrl *ctrl;
	struct atm_vcc *vcc;
	struct sk_buff *skb;
	int err = 0;

	pr_debug("(%d)\n", type);
	if (!atmarpd)
		return -EUNATCH;

	rcu_read_lock();
	vcc = rcu_dereference(atmarpd);
	if (!vcc) {
		err = -EUNATCH;
		goto unlock;
	}
	skb = alloc_skb(sizeof(struct atmarp_ctrl), GFP_ATOMIC);
	if (!skb)
		return -ENOMEM;
	if (!skb) {
		err = -ENOMEM;
		goto unlock;
	}
	ctrl = skb_put(skb, sizeof(struct atmarp_ctrl));
	ctrl->type = type;
	ctrl->itf_num = itf;
	ctrl->ip = ip;
	atm_force_charge(atmarpd, skb->truesize);
	atm_force_charge(vcc, skb->truesize);

	sk = sk_atm(atmarpd);
	sk = sk_atm(vcc);
	skb_queue_tail(&sk->sk_receive_queue, skb);
	sk->sk_data_ready(sk);
	return 0;
unlock:
	rcu_read_unlock();
	return err;
}

static void link_vcc(struct clip_vcc *clip_vcc, struct atmarp_entry *entry)
@@ -417,6 +429,8 @@ static int clip_mkip(struct atm_vcc *vcc, int timeout)

	if (!vcc->push)
		return -EBADFD;
	if (vcc->user_back)
		return -EINVAL;
	clip_vcc = kmalloc(sizeof(struct clip_vcc), GFP_KERNEL);
	if (!clip_vcc)
		return -ENOMEM;
@@ -607,10 +621,12 @@ static void atmarpd_close(struct atm_vcc *vcc)
{
	pr_debug("\n");

	rtnl_lock();
	atmarpd = NULL;
	mutex_lock(&atmarpd_lock);
	RCU_INIT_POINTER(atmarpd, NULL);
	mutex_unlock(&atmarpd_lock);

	synchronize_rcu();
	skb_queue_purge(&sk_atm(vcc)->sk_receive_queue);
	rtnl_unlock();

	pr_debug("(done)\n");
	module_put(THIS_MODULE);
@@ -631,15 +647,18 @@ static struct atm_dev atmarpd_dev = {

static int atm_init_atmarp(struct atm_vcc *vcc)
{
	rtnl_lock();
	if (vcc->push == clip_push)
		return -EINVAL;

	mutex_lock(&atmarpd_lock);
	if (atmarpd) {
		rtnl_unlock();
		mutex_unlock(&atmarpd_lock);
		return -EADDRINUSE;
	}

	mod_timer(&idle_timer, jiffies + CLIP_CHECK_INTERVAL * HZ);

	atmarpd = vcc;
	rcu_assign_pointer(atmarpd, vcc);
	set_bit(ATM_VF_META, &vcc->flags);
	set_bit(ATM_VF_READY, &vcc->flags);
	    /* allow replies and avoid getting closed if signaling dies */
@@ -648,13 +667,14 @@ static int atm_init_atmarp(struct atm_vcc *vcc)
	vcc->push = NULL;
	vcc->pop = NULL; /* crash */
	vcc->push_oam = NULL; /* crash */
	rtnl_unlock();
	mutex_unlock(&atmarpd_lock);
	return 0;
}

static int clip_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
{
	struct atm_vcc *vcc = ATM_SD(sock);
	struct sock *sk = sock->sk;
	int err = 0;

	switch (cmd) {
@@ -675,14 +695,18 @@ static int clip_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
		err = clip_create(arg);
		break;
	case ATMARPD_CTRL:
		lock_sock(sk);
		err = atm_init_atmarp(vcc);
		if (!err) {
			sock->state = SS_CONNECTED;
			__module_get(THIS_MODULE);
		}
		release_sock(sk);
		break;
	case ATMARP_MKIP:
		lock_sock(sk);
		err = clip_mkip(vcc, arg);
		release_sock(sk);
		break;
	case ATMARP_SETENTRY:
		err = clip_setentry(vcc, (__force __be32)arg);