Received-SPF: pass (google.com: domain of linux-kernel+bounces-161427-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <63caf898-8072-48fe-ba7a-2e10e5b2d8ab@web.de>
Date: Sun, 28 Apr 2024 14:52:50 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Markus Elfring <Markus.Elfring@web.de>
Subject: Re: [PATCH V2] scsi: qla2xxx: Fix double free of fcport in error
 handling path
To: Yongzhi Liu <hyperlyzcs@gmail.com>,
 GR-QLogic-Storage-Upstream@marvell.com, linux-scsi@vger.kernel.org,
 kernel-janitors@vger.kernel.org,
 James Bottomley <James.Bottomley@HansenPartnership.com>,
 "Martin K. Petersen" <martin.petersen@oracle.com>,
 Nilesh Javali <njavali@marvell.com>, Saurav Kashyap <skashyap@marvell.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
 Himanshu Madhani <himanshu.madhani@oracle.com>, huntazhang@tencent.com,
 jitxie@tencent.com
References: <443fb75b-948b-430f-be33-170e6f592280@web.de>
 <20240428113404.12522-1-hyperlyzcs@gmail.com>
Content-Language: en-GB
In-Reply-To: <20240428113404.12522-1-hyperlyzcs@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:uN1xlcZIsZ4=;Kp96PXXBw11iC5UJInr9kZBNFbD
 8uGa8Ndl+XmLvn51uh7YA7tsGevYJHBOwl2no8hX441QxuvSd/LBeQT5OfDYb4rvDvS/obKAQ
 T16xCKTl3ectKG6QTp13MJEIN4Fa0654OiAfs4jB1qkb1+ofUHRLTorC1xzxDMBh6gcAjifn0
 PNbg8LKEdXru+OeOKJREsKIVBiqnjqmIg9R/l98I7GYE64IjVdK2kItcJUeXmWzZ5fgoSLc2s
 Zlyx6p5FVxR4FTHsvGcMt/S6eKyD0CKmfB4L+dOfXuJe85FvnlH89QKTkCYlGzw94WJf1WpcQ
 37lvhIn6AYuYt31IZAJSwLuEm7LjjIlXyXEohRkpXHpG/JTVjhuJU6AAY0XRbIpLQAuDqIRJv
 0w9lQC7w9xUiZ8pkpQsWdTG2AFwPd8QRkbURyDKJgWpmz/0u71G7dv99yAO210t4DPKoEYhhV
 g8xroe8aejAb8oG0Y77XTj8loGSKTft1d6OEBv543PYJrvfmNCLdfwbuFULGZPVQkYLxMmMF4
 PO2Hk+Q3nG1311JD8AXIP4OZDxdVSr/HLWi0WOXDGSAABEAYetgEzwucQ3dsAwELBMtZta3sm
 hli9nsYp8tLj5Spm1W7wVzrQJqUcnhjdhmWehrKvkJ78I6yMn4pAUuY+Jqc3Pr35lfuPHRSsL
 zR4c6rOn6Ax/SYr7k49pdo7ep/YGTT+XjjqZ4YVxKc9l0FZxZBS7wtZWney/5qmb3jYDEDG96
 XVcPmBz0/eDK3IUBCx0Q+SlCyzVhM83sG0hc4prtUnr7okqkhO0THEd/489P976GP3nl2XSAq
 gMJQrl5HyzToEFnrxMb7QVjyuD+MrlkidO8QiMkQEEb5E=

=E2=80=A6
> Fix this by cleaning up the redundant qla2x00_free_fcport() and
> replacing error handling with a goto chain.
=E2=80=A6

Can the following wording approach be a bit nicer?

   Thus clean duplicate qla2x00_free_fcport() calls up
   and use more common error handling code instead.


> ---
>  drivers/scsi/qla2xxx/qla_iocb.c | 13 +++++--------
=E2=80=A6

Unfortunately, you overlooked to add a patch version description behind th=
e marker line.

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Do=
cumentation/process/submitting-patches.rst?h=3Dv6.9-rc5#n713


=E2=80=A6
> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
=E2=80=A6
> @@ -2787,6 +2783,7 @@ qla24xx_els_dcmd_iocb(scsi_qla_host_t *vha, int el=
s_opcode,
>
>  	wait_for_completion(&elsio->u.els_logo.comp);
>
> +free_sp:

* I suggest to omit a blank line here.

* How do you think about to use the label =E2=80=9Cput_ref=E2=80=9D?


>  	/* ref: INIT */
>  	kref_put(&sp->cmd_kref, qla2x00_sp_release);
>  	return rval;


Regards,
Markus