All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Skripkin <paskripkin@gmail.com>
To: davem@davemloft.net, kuba@kernel.org, kaber@trash.net
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pavel Skripkin <paskripkin@gmail.com>,
	syzbot+13ad608e190b5f8ad8a8@syzkaller.appspotmail.com
Subject: [PATCH] net: 802: fix memory leak in garp_uninit_applicant
Date: Mon, 19 Jul 2021 00:00:06 +0300	[thread overview]
Message-ID: <20210718210006.26212-1-paskripkin@gmail.com> (raw)

Syzbot reported memory leak in garp_uninit_applicant(). The problem was
in missing clean up function in garp_uninit_applicant().

The reproducer provided by syzbot doing following things in order:

	1. garp_request_join()
 	     garp_attr_event(app, attr, GARP_EVENT_REQ_JOIN);
		/* attr->state == GARP_APPLICANT_VP */

	2. garp_request_leave()
	     garp_attr_event(app, attr, GARP_EVENT_REQ_LEAVE);
		/* attr->state == GARP_APPLICANT_VO */

	3. garp_uninit_applicant()
	     garp_gid_event(app, GARP_EVENT_TRANSMIT_PDU);
  		/* attr is not freed */

Why attr won't be freed? Let's refer to garp_applicant_state_table:

[GARP_APPLICANT_VO] = {
	[GARP_EVENT_TRANSMIT_PDU]	= { .state = GARP_APPLICANT_INVALID },
	[GARP_EVENT_R_JOIN_IN]		= { .state = GARP_APPLICANT_AO },
	[GARP_EVENT_R_JOIN_EMPTY]	= { .state = GARP_APPLICANT_VO },
	[GARP_EVENT_R_EMPTY]		= { .state = GARP_APPLICANT_VO },
	[GARP_EVENT_R_LEAVE_IN]		= { .state = GARP_APPLICANT_VO },
	[GARP_EVENT_R_LEAVE_EMPTY]	= { .state = GARP_APPLICANT_VO },
	[GARP_EVENT_REQ_JOIN]		= { .state = GARP_APPLICANT_VP },
	[GARP_EVENT_REQ_LEAVE]		= { .state = GARP_APPLICANT_INVALID },

REQ_LEAVE event has INVALID state as standard says and
garp_attr_event() just returns in case of invalid state.
Since garp_uninit_applicant() is destroy function for applicant we need
to free remaining attrs to avoid memory leaks.

Fixes: eca9ebac651f ("net: Add GARP applicant-only participant")
Reported-and-tested-by: syzbot+13ad608e190b5f8ad8a8@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 net/802/garp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/802/garp.c b/net/802/garp.c
index 400bd857e5f5..4a1ef95ae428 100644
--- a/net/802/garp.c
+++ b/net/802/garp.c
@@ -553,6 +553,16 @@ static void garp_release_port(struct net_device *dev)
 	kfree_rcu(port, rcu);
 }
 
+static void garp_destroy_remaining_attrs(struct garp_applicant *app)
+{
+	while (!RB_EMPTY_ROOT(&app->gid)) {
+		struct garp_attr *attr =
+			rb_entry(rb_first(&app->gid),
+				 struct garp_attr, node);
+		garp_attr_destroy(app, attr);
+	}
+}
+
 int garp_init_applicant(struct net_device *dev, struct garp_application *appl)
 {
 	struct garp_applicant *app;
@@ -610,6 +620,13 @@ void garp_uninit_applicant(struct net_device *dev, struct garp_application *appl
 	spin_lock_bh(&app->lock);
 	garp_gid_event(app, GARP_EVENT_TRANSMIT_PDU);
 	garp_pdu_queue(app);
+
+	/* We need to free remaining attrs since this scenario is possible:
+	 *	garp_request_join()
+	 *	garp_request_leave()
+	 *	garp_uninit_applicant()
+	 */
+	garp_destroy_remaining_attrs(app);
 	spin_unlock_bh(&app->lock);
 
 	garp_queue_xmit(app);
-- 
2.32.0


             reply	other threads:[~2021-07-18 21:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-18 21:00 Pavel Skripkin [this message]
2021-07-19 10:05 ` [PATCH] net: 802: fix memory leak in garp_uninit_applicant Jakub Kicinski
2021-07-19 10:29   ` Pavel Skripkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210718210006.26212-1-paskripkin@gmail.com \
    --to=paskripkin@gmail.com \
    --cc=davem@davemloft.net \
    --cc=kaber@trash.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=syzbot+13ad608e190b5f8ad8a8@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.