linux-kernel - Re: [PATCH RESEND 2/2] scsi: libfc: drop extra rport reference in fc_rport

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: <cdaf7362-b0dd-de99-b7ec-318b60b85192@suse.de>
Date:   Fri, 21 Feb 2020 10:21:36 +0100
From:   Hannes Reinecke <hare@...e.de>
To:     Igor Druzhinin <igor.druzhinin@...rix.com>,
        fcoe-devel@...n-fcoe.org, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     jejb@...ux.ibm.com, martin.petersen@...cle.com
Subject: Re: [PATCH RESEND 2/2] scsi: libfc: drop extra rport reference in
 fc_rport_create()

On 1/14/20 3:43 PM, Igor Druzhinin wrote:
> The callers of this function seem to assume the reference is not taken
> in case rport already exists. This results in one extra reference taken
> on each rport re-discovery that will eventually get to inability to
> free rport structure on port removal.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@...rix.com>
> ---
>  drivers/scsi/libfc/fc_rport.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> index da6e97d..a43f9dd 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -133,8 +133,10 @@ struct fc_rport_priv *fc_rport_create(struct fc_lport *lport, u32 port_id)
>  	lockdep_assert_held(&lport->disc.disc_mutex);
>  
>  	rdata = fc_rport_lookup(lport, port_id);
> -	if (rdata)
> +	if (rdata) {
> +		kref_put(&rdata->kref, fc_rport_destroy);
>  		return rdata;
> +	}
>  
>  	if (lport->rport_priv_size > 0)
>  		rport_priv_size = lport->rport_priv_size;
> 
NAK.
The caller _does_ assume that a reference is taken once
fc_rport_create() returns non-NULL.
And the caller is responsible to drop the reference once 'rdatat' isn't
used anymore.
Any other usage is an error, but should be fixed in the caller, not here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@...e.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer