Messages in this thread Patch in this message | | | From | Pavel Skripkin <> | Subject | [PATCH] net: 802: fix memory leak in mrp_uninit_applicant | Date | Mon, 19 Jul 2021 00:01:04 +0300 |
| |
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 <paskripkin@gmail.com> --- 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
| |