Hi, On Wed, Apr 14, 2021 at 12:08:32AM +0800, lyl2019@xxxxxxxxxxxxxxxx wrote: > > Hi, > maintianers. > > Sorry to disturb you, but this patch seems to be missed more than two weeks. > Could you help to review this patch? I am sure it won't take you much time. You might take a look here: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#checking_state_of_patches_from_patchwork https://patchwork.kernel.org/project/linux-wireless/list/ Two weeks is not unheard of for waiting. Anyway, you *did* succeed in catching my attention today, so I'll give you a review, below: > > -----原始邮件----- > > 发件人: "Lv Yunlong" <lyl2019@xxxxxxxxxxxxxxxx> > > 发送时间: 2021-03-29 19:24:35 (星期一) > > 收件人: amitkarwar@xxxxxxxxx, ganapathi.bhat@xxxxxxx, huxinming820@xxxxxxxxx, kvalo@xxxxxxxxxxxxxx, davem@xxxxxxxxxxxxx, kuba@xxxxxxxxxx > > 抄送: linux-wireless@xxxxxxxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, "Lv Yunlong" <lyl2019@xxxxxxxxxxxxxxxx> > > 主题: [PATCH] wireless/marvell/mwifiex: Fix a double free in mwifiex_send_tdls_action_frame > > > > In mwifiex_send_tdls_action_frame, it calls mwifiex_construct_tdls_action_frame > > (..,skb). The skb will be freed in mwifiex_construct_tdls_action_frame() when > > it is failed. But when mwifiex_construct_tdls_action_frame() returns error, > > the skb will be freed in the second time by dev_kfree_skb_any(skb). > > > > My patch removes the redundant dev_kfree_skb_any(skb) when > > mwifiex_construct_tdls_action_frame() failed. > > > > Fixes: b23bce2965680 ("mwifiex: add tdls_mgmt handler support") > > Signed-off-by: Lv Yunlong <lyl2019@xxxxxxxxxxxxxxxx> > > --- > > drivers/net/wireless/marvell/mwifiex/tdls.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c b/drivers/net/wireless/marvell/mwifiex/tdls.c > > index 97bb87c3676b..8d4d0a9cf6ac 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/tdls.c > > +++ b/drivers/net/wireless/marvell/mwifiex/tdls.c > > @@ -856,7 +856,6 @@ int mwifiex_send_tdls_action_frame(struct mwifiex_private *priv, const u8 *peer, > > if (mwifiex_construct_tdls_action_frame(priv, peer, action_code, > > dialog_token, status_code, > > skb)) { > > - dev_kfree_skb_any(skb); Good catch, and this looks correct for most cases, but I'll note that you missed one issue: mwifiex_construct_tdls_action_frame() has a default/error case ("Unknown TDLS action frame type") which does *not* free this SKB, so you should patch that up at the same time. Otherwise, you're trading a double-free for a potential memory leak. With that fixed: Reviewed-by: Brian Norris <briannorris@xxxxxxxxxxxx> Thanks. > > return -EINVAL; > > } > > > > -- > > 2.25.1 > >
