netdev - RE: [PATCH] qed: Fix to use list_for_each_entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BL2PR07MB2306DE6412B6516663F3BE608DD80@BL2PR07MB2306.namprd07.prod.outlook.com>
Date:   Sun, 9 Oct 2016 13:44:42 +0000
From:   "Mintz, Yuval" <Yuval.Mintz@...ium.com>
To:     Wei Yongjun <weiyj.lk@...il.com>,
        Ariel Elior <Ariel.Elior@...gic.com>
CC:     Wei Yongjun <weiyongjun1@...wei.com>,
        "everest-linux-l2@...gic.com" <everest-linux-l2@...gic.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH] qed: Fix to use list_for_each_entry_safe() when delete
 items

> From: Wei Yongjun <weiyongjun1@...wei.com>
> 
> Since we will remove items off the list using list_del() we need to use a safe
> version of the list_for_each_entry() macro aptly named
> list_for_each_entry_safe().
> 
> Fixes: 0a7fb11c23c0 ("qed: Add Light L2 support")
> Signed-off-by: Wei Yongjun <weiyongjun1@...wei.com>
> ---
>  drivers/net/ethernet/qlogic/qed/qed_ll2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> index a6db107..4428333 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
> @@ -1517,7 +1517,7 @@ static void qed_ll2_register_cb_ops(struct qed_dev
> *cdev,  static int qed_ll2_start(struct qed_dev *cdev, struct qed_ll2_params
> *params)  {
>  	struct qed_ll2_info ll2_info;
> -	struct qed_ll2_buffer *buffer;
> +	struct qed_ll2_buffer *buffer, *tmp;
>  	enum qed_ll2_conn_type conn_type;
>  	struct qed_ptt *p_ptt;
>  	int rc, i;
> @@ -1587,7 +1587,7 @@ static int qed_ll2_start(struct qed_dev *cdev, struct
> qed_ll2_params *params)
> 
>  	/* Post all Rx buffers to FW */
>  	spin_lock_bh(&cdev->ll2->lock);
> -	list_for_each_entry(buffer, &cdev->ll2->list, list) {
> +	list_for_each_entry_safe(buffer, tmp, &cdev->ll2->list, list) {
>  		rc = qed_ll2_post_rx_buffer(QED_LEADING_HWFN(cdev),
>  					    cdev->ll2->handle,
>  					    buffer->phys_addr, 0, buffer, 1);

Thanks for the catch.

A single petty comment - having the variable called 'tmp' in such a long
function is far from informative. Perhaps 'tmp_buffer' would have been
better.

Regardless,
Acked-by: Yuval Mintz <Yuval.Mintz@...iumnetworks.com>