| Message ID | 20241029125600.3036659-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net: wwan: t7xx: Fix off-by-one error in t7xx_dpmaif_rx_buf_alloc() | expand |
On October 29, 2024 2:56:00 PM, Jinjie Ruan <ruanjinjie@huawei.com> wrote: >The error path in t7xx_dpmaif_rx_buf_alloc(), free and unmap the already >allocated and mapped skb in a loop, but the loop condition terminates when >the index reaches zero, which fails to free the first allocated skb at >index zero. > >Check for >= 0 so that skb at index 0 is freed as well. > >Fixes: d642b012df70 ("net: wwan: t7xx: Add data path interface") >Suggested-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> >Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Jakub, when applying, could you drop that suggested-by tag, please. My contribution was only a small suggestion to avoid an endless loop. In all other meanings this patch is original work made by Jinjie, and all creds should go to him. Besides that tag: Acked-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> >--- >v2: >- Update the commit title. >- Declare i as signed to avoid the endless loop. >--- > drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c >index 210d84c67ef9..45e7833965b1 100644 >--- a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c >+++ b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c >@@ -166,8 +166,8 @@ int t7xx_dpmaif_rx_buf_alloc(struct dpmaif_ctrl *dpmaif_ctrl, > const unsigned int q_num, const unsigned int buf_cnt, > const bool initial) > { >- unsigned int i, bat_cnt, bat_max_cnt, bat_start_idx; >- int ret; >+ unsigned int bat_cnt, bat_max_cnt, bat_start_idx; >+ int ret, i; > > if (!buf_cnt || buf_cnt > bat_req->bat_size_cnt) > return -EINVAL; >@@ -226,7 +226,7 @@ int t7xx_dpmaif_rx_buf_alloc(struct dpmaif_ctrl *dpmaif_ctrl, > return 0; > > err_unmap_skbs: >- while (--i > 0) >+ while (--i >= 0) > t7xx_unmap_bat_skb(dpmaif_ctrl->dev, bat_req->bat_skb, i); > > return ret;
On Wed, 30 Oct 2024 18:01:07 +0200 Sergey Ryazanov wrote: > On October 29, 2024 2:56:00 PM, Jinjie Ruan <ruanjinjie@huawei.com> wrote: > >The error path in t7xx_dpmaif_rx_buf_alloc(), free and unmap the already > >allocated and mapped skb in a loop, but the loop condition terminates when > >the index reaches zero, which fails to free the first allocated skb at > >index zero. > > > >Check for >= 0 so that skb at index 0 is freed as well. > > > >Fixes: d642b012df70 ("net: wwan: t7xx: Add data path interface") > >Suggested-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> > >Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > > Jakub, when applying, could you drop that suggested-by tag, please. > My contribution was only a small suggestion to avoid an endless loop. > In all other meanings this patch is original work made by Jinjie, and > all creds should go to him. TBH I find the while (i--) solution suggested by Ilpo more idiomatic, too, let's do a v3 with that and without unnecessary Suggested-by tags.
diff --git a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c index 210d84c67ef9..45e7833965b1 100644 --- a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c +++ b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c @@ -166,8 +166,8 @@ int t7xx_dpmaif_rx_buf_alloc(struct dpmaif_ctrl *dpmaif_ctrl, const unsigned int q_num, const unsigned int buf_cnt, const bool initial) { - unsigned int i, bat_cnt, bat_max_cnt, bat_start_idx; - int ret; + unsigned int bat_cnt, bat_max_cnt, bat_start_idx; + int ret, i; if (!buf_cnt || buf_cnt > bat_req->bat_size_cnt) return -EINVAL; @@ -226,7 +226,7 @@ int t7xx_dpmaif_rx_buf_alloc(struct dpmaif_ctrl *dpmaif_ctrl, return 0; err_unmap_skbs: - while (--i > 0) + while (--i >= 0) t7xx_unmap_bat_skb(dpmaif_ctrl->dev, bat_req->bat_skb, i); return ret;
The error path in t7xx_dpmaif_rx_buf_alloc(), free and unmap the already allocated and mapped skb in a loop, but the loop condition terminates when the index reaches zero, which fails to free the first allocated skb at index zero. Check for >= 0 so that skb at index 0 is freed as well. Fixes: d642b012df70 ("net: wwan: t7xx: Add data path interface") Suggested-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- v2: - Update the commit title. - Declare i as signed to avoid the endless loop. --- drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)