Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6c6018bb-2d28-1bd7-1d91-fbec47373c79@redhat.com>
Date:   Mon, 24 Oct 2022 13:38:12 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH] blkmapd: fix coredump in bl_add_disk
Content-Language: en-US
To:     lixiaokeng <lixiaokeng@huawei.com>, linux-nfs@vger.kernel.org
Cc:     "liuzhiqiang (I)" <liuzhiqiang26@huawei.com>,
        linfeilong <linfeilong@huawei.com>
References: <77a09978-a5aa-ea7f-04b8-a8d398ee325f@huawei.com>
From:   Steve Dickson <steved@redhat.com>
In-Reply-To: <77a09978-a5aa-ea7f-04b8-a8d398ee325f@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/1/21 3:21 AM, lixiaokeng wrote:
> The serial->data is not malloced separately (just part of
> the serial), so it can't be freed. The bl_serial has its
> own free function. Use it.
> 
> Signed-off-by: Lixiaokeng <lixiaokeng@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Committed...

steved.

> ---
>   utils/blkmapd/device-discovery.c | 15 +++------------
>   utils/blkmapd/device-discovery.h |  2 ++
>   utils/blkmapd/device-inq.c       |  4 ++--
>   3 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
> index 2736ac89..cea33496 100644
> --- a/utils/blkmapd/device-discovery.c
> +++ b/utils/blkmapd/device-discovery.c
> @@ -187,10 +187,7 @@ static void bl_add_disk(char *filepath)
>   	}
> 
>   	if (disk && diskpath) {
> -		if (serial) {
> -			free(serial->data);
> -			free(serial);
> -		}
> +		bl_free_scsi_string(serial);
>   		return;
>   	}
> 
> @@ -228,10 +225,7 @@ static void bl_add_disk(char *filepath)
>   			disk->size = size;
>   			disk->valid_path = path;
>   		}
> -		if (serial) {
> -			free(serial->data);
> -			free(serial);
> -		}
> +		bl_free_scsi_string(serial);
>   	}
>   	return;
> 
> @@ -241,10 +235,7 @@ static void bl_add_disk(char *filepath)
>   			free(path->full_path);
>   		free(path);
>   	}
> -	if (serial) {
> -		free(serial->data);
> -		free(serial);
> -	}
> +	bl_free_scsi_string(serial);
>   	return;
>   }
> 
> diff --git a/utils/blkmapd/device-discovery.h b/utils/blkmapd/device-discovery.h
> index a86eed99..462aa943 100644
> --- a/utils/blkmapd/device-discovery.h
> +++ b/utils/blkmapd/device-discovery.h
> @@ -151,6 +151,8 @@ uint64_t process_deviceinfo(const char *dev_addr_buf,
> 
>   extern ssize_t atomicio(ssize_t(*f) (int, void *, size_t),
>   			int fd, void *_s, size_t n);
> +extern struct bl_serial *bl_create_scsi_string(int len, const char *bytes);
> +extern void bl_free_scsi_string(struct bl_serial *str);
>   extern struct bl_serial *bldev_read_serial(int fd, const char *filename);
>   extern enum bl_path_state_e bldev_read_ap_state(int fd);
>   extern int bl_discover_devices(void);
> diff --git a/utils/blkmapd/device-inq.c b/utils/blkmapd/device-inq.c
> index c7952c3e..9e5749ef 100644
> --- a/utils/blkmapd/device-inq.c
> +++ b/utils/blkmapd/device-inq.c
> @@ -53,7 +53,7 @@
>   #define DEF_ALLOC_LEN	255
>   #define MX_ALLOC_LEN	(0xc000 + 0x80)
> 
> -static struct bl_serial *bl_create_scsi_string(int len, const char *bytes)
> +struct bl_serial *bl_create_scsi_string(int len, const char *bytes)
>   {
>   	struct bl_serial *s;
> 
> @@ -66,7 +66,7 @@ static struct bl_serial *bl_create_scsi_string(int len, const char *bytes)
>   	return s;
>   }
> 
> -static void bl_free_scsi_string(struct bl_serial *str)
> +void bl_free_scsi_string(struct bl_serial *str)
>   {
>   	if (str)
>   		free(str);