Messages in this thread |  | | Date | Sun, 2 May 2021 20:00:20 +0800 (GMT+08:00) | From | lyl2019@mail ... | Subject | Re: Re: [PATCH] ethernet:enic: Fix a use after free bug in enic_hard_start_xmit |
| |
> -----原始邮件----- > 发件人: "Govindarajulu Varadarajan (gvaradar)" <gvaradar@cisco.com> > 发送时间: 2021-05-02 09:12:49 (星期日) > 收件人: "Christian Benvenuti (benve)" <benve@cisco.com>, "davem@davemloft.net" <davem@davemloft.net>, "kuba@kernel.org" <kuba@kernel.org>, "lyl2019@mail.ustc.edu.cn" <lyl2019@mail.ustc.edu.cn> > 抄送: "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Govindarajulu > Varadarajan (gvaradar)" <gvaradar@cisco.com> > 主题: Re: [PATCH] ethernet:enic: Fix a use after free bug in enic_hard_start_xmit > > On Sat, 2021-05-01 at 08:31 -0700, Lv Yunlong wrote: > > In enic_hard_start_xmit, it calls enic_queue_wq_skb(). Inside > > enic_queue_wq_skb, if some error happens, the skb will be freed > > by dev_kfree_skb(skb). But the freed skb is still used in > > skb_tx_timestamp(skb). > > > > My patch makes enic_queue_wq_skb() return error and goto spin_unlock() > > incase of error. The solution is provided by Govind. > > See https://lkml.org/lkml/2021/4/30/961. > > > > Fixes: fb7516d42478e ("enic: add sw timestamp support") > > Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> > > --- > > drivers/net/ethernet/cisco/enic/enic_main.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c > > b/drivers/net/ethernet/cisco/enic/enic_main.c > > index f04ec53544ae..40abc3fdeba6 100644 > > --- a/drivers/net/ethernet/cisco/enic/enic_main.c > > +++ b/drivers/net/ethernet/cisco/enic/enic_main.c > > @@ -768,7 +768,7 @@ static inline int enic_queue_wq_skb_encap(struct enic > > *enic, struct vnic_wq *wq, > > return err; > > } > > > > -static inline void enic_queue_wq_skb(struct enic *enic, > > +static inline int enic_queue_wq_skb(struct enic *enic, > > struct vnic_wq *wq, struct sk_buff *skb) > > { > > unsigned int mss = skb_shinfo(skb)->gso_size; > > @@ -813,7 +813,9 @@ static inline void enic_queue_wq_skb(struct enic *enic, > > } > > wq->to_use = buf->next; > > dev_kfree_skb(skb); > > + return err; > > return err seems unnecessary here. > > > } > > + return err; > > } > > > > /* netif_tx_lock held, process context with BHs disabled, or BH */ > > @@ -857,7 +859,8 @@ static netdev_tx_t enic_hard_start_xmit(struct sk_buff > > *skb, > > return NETDEV_TX_BUSY; > > } > > > > - enic_queue_wq_skb(enic, wq, skb); > > + if (enic_queue_wq_skb(enic, wq, skb) < 0) > > 0 is success, any other value is error. if (enic_queue_wq_skb(enic, wq, skb)). > > Otherwise patch looks good. > > Thanks > Govind
Thank you. I have corrected my patch. See https://lore.kernel.org/patchwork/patch/1420731/.
|  |