[PATCH] net: 802: fix memory leak in mrp_uninit_applicant
From: Pavel Skripkin <hidden>
Date: 2021-07-18 21:01:16
Also in:
lkml
Subsystem:
networking [general], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
Syzbot reported memory leak in mrp_uninit_applicant(). The problem was
in missing clean up function in mrp_uninit_applicant().
The reproducer provided by syzbot doing following things in order:
1. mrp_request_join()
mrp_attr_event(app, attr, MRP_EVENT_JOIN);
/* attr->state == MRP_APPLICANT_VP */
2. mrp_request_leave()
mrp_attr_event(app, attr, MRP_EVENT_LV);
/* attr->state == MRP_APPLICANT_VO */
3. mrp_uninit_applicant()
mrp_mad_event(app, MRP_EVENT_TX);
/* attr is not freed */
Why attr won't be freed? Since last event == MRP_EVENT_TX let's refer
to mrp_tx_action_table:
static const u8
mrp_tx_action_table[MRP_APPLICANT_MAX + 1] = {
[MRP_APPLICANT_VO] = MRP_TX_ACTION_S_IN_OPTIONAL,
[MRP_APPLICANT_VP] = MRP_TX_ACTION_S_JOIN_IN,
[MRP_APPLICANT_VN] = MRP_TX_ACTION_S_NEW,
[MRP_APPLICANT_AN] = MRP_TX_ACTION_S_NEW,
[MRP_APPLICANT_AA] = MRP_TX_ACTION_S_JOIN_IN,
[MRP_APPLICANT_QA] = MRP_TX_ACTION_S_JOIN_IN_OPTIONAL,
[MRP_APPLICANT_LA] = MRP_TX_ACTION_S_LV,
[MRP_APPLICANT_AO] = MRP_TX_ACTION_S_IN_OPTIONAL,
[MRP_APPLICANT_QO] = MRP_TX_ACTION_S_IN_OPTIONAL,
[MRP_APPLICANT_AP] = MRP_TX_ACTION_S_JOIN_IN,
[MRP_APPLICANT_QP] = MRP_TX_ACTION_S_IN_OPTIONAL,
};
[MRP_APPLICANT_VO] member has MRP_TX_ACTION_S_IN_OPTIONAL action and
mrp_attr_event() just returns in case of this action.
Since mrp_uninit_applicant() is destroy function for applicant we need
to free remaining attrs to avoid memory leaks.
Reported-and-tested-by: syzbot+5cfab121b54dff775399@syzkaller.appspotmail.com
Fixes: febf018d2234 ("net/802: Implement Multiple Registration Protocol (MRP)")
Signed-off-by: Pavel Skripkin <redacted>
---
net/802/mrp.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/net/802/mrp.c b/net/802/mrp.c
index bea6e43d45a0..bf319f3f2094 100644
--- a/net/802/mrp.c
+++ b/net/802/mrp.c@@ -834,6 +834,16 @@ static void mrp_release_port(struct net_device *dev) kfree_rcu(port, rcu); } +static void mrp_destroy_remaining_attrs(struct mrp_applicant *app) +{ + while (!RB_EMPTY_ROOT(&app->mad)) { + struct mrp_attr *attr = + rb_entry(rb_first(&app->mad), + struct mrp_attr, node); + mrp_attr_destroy(app, attr); + } +} + int mrp_init_applicant(struct net_device *dev, struct mrp_application *appl) { struct mrp_applicant *app;
@@ -896,6 +906,13 @@ void mrp_uninit_applicant(struct net_device *dev, struct mrp_application *appl) spin_lock_bh(&app->lock); mrp_mad_event(app, MRP_EVENT_TX); mrp_pdu_queue(app); + + /* We need to free remaining attrs since this scenario is possible: + * mrp_request_join() + * mrp_request_leave() + * mrp_uninit_applicant() + */ + mrp_destroy_remaining_attrs(app); spin_unlock_bh(&app->lock); mrp_queue_xmit(app);
--
2.32.0