* [PATCH] b2iscsi: Fix memory leak in mgmt_set_ip()
@ 2014-06-27 12:55 Maurizio Lombardi
2014-06-27 16:32 ` Tomas Henzl
0 siblings, 1 reply; 6+ messages in thread
From: Maurizio Lombardi @ 2014-06-27 12:55 UTC (permalink / raw)
To: jayamohan.kallickal; +Cc: linux-scsi, michaelc, hch, JBottomley
The if_info pointer is not released by the mgmt_set_ip() function
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
drivers/scsi/be2iscsi/be_mgmt.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index 07934b0..a3e5648 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -1015,7 +1015,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
if (if_info->dhcp_state) {
beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
"BG_%d : DHCP Already Enabled\n");
- return 0;
+ goto exit;
}
/* The ip_param->len is 1 in DHCP case. Setting
proper IP len as this it is used while
@@ -1033,7 +1033,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
sizeof(*reldhcp));
if (rc)
- return rc;
+ goto exit;
reldhcp = nonemb_cmd.va;
reldhcp->interface_hndl = phba->interface_handle;
@@ -1044,7 +1044,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
beiscsi_log(phba, KERN_WARNING,
BEISCSI_LOG_CONFIG,
"BG_%d : Failed to Delete existing dhcp\n");
- return rc;
+ goto exit;
}
}
}
@@ -1054,7 +1054,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
rc = mgmt_static_ip_modify(phba, if_info, ip_param, NULL,
IP_ACTION_DEL);
if (rc)
- return rc;
+ goto exit;
}
/* Delete the Gateway settings if mode change is to DHCP */
@@ -1064,7 +1064,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
if (rc) {
beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
"BG_%d : Failed to Get Gateway Addr\n");
- return rc;
+ goto exit;
}
if (gtway_addr_set.ip_addr.addr[0]) {
@@ -1076,7 +1076,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
beiscsi_log(phba, KERN_WARNING,
BEISCSI_LOG_CONFIG,
"BG_%d : Failed to clear Gateway Addr Set\n");
- return rc;
+ goto exit;
}
}
}
@@ -1087,7 +1087,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
OPCODE_COMMON_ISCSI_NTWK_CONFIG_STATELESS_IP_ADDR,
sizeof(*dhcpreq));
if (rc)
- return rc;
+ goto exit;
dhcpreq = nonemb_cmd.va;
dhcpreq->flags = BLOCKING;
@@ -1095,12 +1095,14 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
dhcpreq->interface_hndl = phba->interface_handle;
dhcpreq->ip_type = BE2_DHCP_V4;
- return mgmt_exec_nonemb_cmd(phba, &nonemb_cmd, NULL, 0);
+ rc = mgmt_exec_nonemb_cmd(phba, &nonemb_cmd, NULL, 0);
} else {
- return mgmt_static_ip_modify(phba, if_info, ip_param,
+ rc = mgmt_static_ip_modify(phba, if_info, ip_param,
subnet_param, IP_ACTION_ADD);
}
+exit:
+ kfree(if_info);
return rc;
}
--
Maurizio Lombardi
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] b2iscsi: Fix memory leak in mgmt_set_ip()
2014-06-27 12:55 [PATCH] b2iscsi: Fix memory leak in mgmt_set_ip() Maurizio Lombardi
@ 2014-06-27 16:32 ` Tomas Henzl
2014-06-27 19:38 ` Maurizio Lombardi
0 siblings, 1 reply; 6+ messages in thread
From: Tomas Henzl @ 2014-06-27 16:32 UTC (permalink / raw)
To: Maurizio Lombardi, jayamohan.kallickal
Cc: linux-scsi, michaelc, hch, JBottomley
On 06/27/2014 02:55 PM, Maurizio Lombardi wrote:
> The if_info pointer is not released by the mgmt_set_ip() function
>
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> ---
> drivers/scsi/be2iscsi/be_mgmt.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
> index 07934b0..a3e5648 100644
> --- a/drivers/scsi/be2iscsi/be_mgmt.c
> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
> @@ -1015,7 +1015,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> if (if_info->dhcp_state) {
> beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
> "BG_%d : DHCP Already Enabled\n");
> - return 0;
> + goto exit;
rc should be initialised - so this needs to be added to your patch
Cheers, Tomas
@@ -955,7 +955,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
struct be_dma_mem nonemb_cmd;
uint8_t *gtway_addr;
uint32_t ip_type;
- int rc;
+ int rc = 0;
if (mgmt_get_all_if_id(phba))
return -EIO;
> }
> /* The ip_param->len is 1 in DHCP case. Setting
> proper IP len as this it is used while
> @@ -1033,7 +1033,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> sizeof(*reldhcp));
>
> if (rc)
> - return rc;
> + goto exit;
>
> reldhcp = nonemb_cmd.va;
> reldhcp->interface_hndl = phba->interface_handle;
> @@ -1044,7 +1044,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> beiscsi_log(phba, KERN_WARNING,
> BEISCSI_LOG_CONFIG,
> "BG_%d : Failed to Delete existing dhcp\n");
> - return rc;
> + goto exit;
> }
> }
> }
> @@ -1054,7 +1054,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> rc = mgmt_static_ip_modify(phba, if_info, ip_param, NULL,
> IP_ACTION_DEL);
> if (rc)
> - return rc;
> + goto exit;
> }
>
> /* Delete the Gateway settings if mode change is to DHCP */
> @@ -1064,7 +1064,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> if (rc) {
> beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
> "BG_%d : Failed to Get Gateway Addr\n");
> - return rc;
> + goto exit;
> }
>
> if (gtway_addr_set.ip_addr.addr[0]) {
> @@ -1076,7 +1076,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> beiscsi_log(phba, KERN_WARNING,
> BEISCSI_LOG_CONFIG,
> "BG_%d : Failed to clear Gateway Addr Set\n");
> - return rc;
> + goto exit;
> }
> }
> }
> @@ -1087,7 +1087,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> OPCODE_COMMON_ISCSI_NTWK_CONFIG_STATELESS_IP_ADDR,
> sizeof(*dhcpreq));
> if (rc)
> - return rc;
> + goto exit;
>
> dhcpreq = nonemb_cmd.va;
> dhcpreq->flags = BLOCKING;
> @@ -1095,12 +1095,14 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> dhcpreq->interface_hndl = phba->interface_handle;
> dhcpreq->ip_type = BE2_DHCP_V4;
>
> - return mgmt_exec_nonemb_cmd(phba, &nonemb_cmd, NULL, 0);
> + rc = mgmt_exec_nonemb_cmd(phba, &nonemb_cmd, NULL, 0);
> } else {
> - return mgmt_static_ip_modify(phba, if_info, ip_param,
> + rc = mgmt_static_ip_modify(phba, if_info, ip_param,
> subnet_param, IP_ACTION_ADD);
> }
>
> +exit:
> + kfree(if_info);
> return rc;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] b2iscsi: Fix memory leak in mgmt_set_ip()
2014-06-27 16:32 ` Tomas Henzl
@ 2014-06-27 19:38 ` Maurizio Lombardi
2014-06-29 18:28 ` Tomas Henzl
0 siblings, 1 reply; 6+ messages in thread
From: Maurizio Lombardi @ 2014-06-27 19:38 UTC (permalink / raw)
To: Tomas Henzl, jayamohan.kallickal; +Cc: linux-scsi, michaelc, hch, JBottomley
>> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
>> index 07934b0..a3e5648 100644
>> --- a/drivers/scsi/be2iscsi/be_mgmt.c
>> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
>> @@ -1015,7 +1015,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
>> if (if_info->dhcp_state) {
>> beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
>> "BG_%d : DHCP Already Enabled\n");
>> - return 0;
>> + goto exit;
>
> rc should be initialised - so this needs to be added to your patch
It is initialized at the beginning of the function, and it must contain zero when the "goto exit" is executed.
int mgmt_set_ip(struct beiscsi_hba *phba,
struct iscsi_iface_param_info *ip_param,
struct iscsi_iface_param_info *subnet_param,
uint32_t boot_proto)
{
struct be_cmd_get_def_gateway_resp gtway_addr_set;
struct be_cmd_get_if_info_resp *if_info;
struct be_cmd_set_dhcp_req *dhcpreq;
struct be_cmd_rel_dhcp_req *reldhcp;
struct be_dma_mem nonemb_cmd;
uint8_t *gtway_addr;
uint32_t ip_type;
int rc;
if (mgmt_get_all_if_id(phba))
return -EIO;
ip_type = (ip_param->param == ISCSI_NET_PARAM_IPV6_ADDR) ?
BE2_IPV6 : BE2_IPV4 ;
rc = mgmt_get_if_info(phba, ip_type, &if_info); <-------------- here
if (rc)
return rc;
if (boot_proto == ISCSI_BOOTPROTO_DHCP) {
if (if_info->dhcp_state) {
beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
"BG_%d : DHCP Already Enabled\n");
goto exit;
}
Regards,
Maurizio Lombardi
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] b2iscsi: Fix memory leak in mgmt_set_ip()
2014-06-27 19:38 ` Maurizio Lombardi
@ 2014-06-29 18:28 ` Tomas Henzl
2014-07-01 18:53 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Tomas Henzl @ 2014-06-29 18:28 UTC (permalink / raw)
To: Maurizio Lombardi, jayamohan.kallickal
Cc: linux-scsi, michaelc, hch, JBottomley
On 06/27/2014 09:38 PM, Maurizio Lombardi wrote:
>
>>> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
>>> index 07934b0..a3e5648 100644
>>> --- a/drivers/scsi/be2iscsi/be_mgmt.c
>>> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
>>> @@ -1015,7 +1015,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
>>> if (if_info->dhcp_state) {
>>> beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
>>> "BG_%d : DHCP Already Enabled\n");
>>> - return 0;
>>> + goto exit;
>> rc should be initialised - so this needs to be added to your patch
> It is initialized at the beginning of the function, and it must contain zero when the "goto exit" is executed.
Correct, I haven't noticed the initialisation, sorry for the confusion.
--tm
>
> int mgmt_set_ip(struct beiscsi_hba *phba,
> struct iscsi_iface_param_info *ip_param,
> struct iscsi_iface_param_info *subnet_param,
> uint32_t boot_proto)
> {
> struct be_cmd_get_def_gateway_resp gtway_addr_set;
> struct be_cmd_get_if_info_resp *if_info;
> struct be_cmd_set_dhcp_req *dhcpreq;
> struct be_cmd_rel_dhcp_req *reldhcp;
> struct be_dma_mem nonemb_cmd;
> uint8_t *gtway_addr;
> uint32_t ip_type;
> int rc;
>
> if (mgmt_get_all_if_id(phba))
> return -EIO;
>
> ip_type = (ip_param->param == ISCSI_NET_PARAM_IPV6_ADDR) ?
> BE2_IPV6 : BE2_IPV4 ;
>
> rc = mgmt_get_if_info(phba, ip_type, &if_info); <-------------- here
> if (rc)
> return rc;
>
> if (boot_proto == ISCSI_BOOTPROTO_DHCP) {
> if (if_info->dhcp_state) {
> beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
> "BG_%d : DHCP Already Enabled\n");
> goto exit;
> }
>
> Regards,
> Maurizio Lombardi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] b2iscsi: Fix memory leak in mgmt_set_ip()
2014-06-29 18:28 ` Tomas Henzl
@ 2014-07-01 18:53 ` Christoph Hellwig
2014-07-07 11:29 ` Tomas Henzl
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2014-07-01 18:53 UTC (permalink / raw)
To: Tomas Henzl
Cc: Maurizio Lombardi, jayamohan.kallickal, linux-scsi, michaelc, hch,
JBottomley
On Sun, Jun 29, 2014 at 08:28:34PM +0200, Tomas Henzl wrote:
> On 06/27/2014 09:38 PM, Maurizio Lombardi wrote:
> >
> >>> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
> >>> index 07934b0..a3e5648 100644
> >>> --- a/drivers/scsi/be2iscsi/be_mgmt.c
> >>> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
> >>> @@ -1015,7 +1015,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
> >>> if (if_info->dhcp_state) {
> >>> beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
> >>> "BG_%d : DHCP Already Enabled\n");
> >>> - return 0;
> >>> + goto exit;
> >> rc should be initialised - so this needs to be added to your patch
> > It is initialized at the beginning of the function, and it must contain zero when the "goto exit" is executed.
>
> Correct, I haven't noticed the initialisation, sorry for the confusion.
Is this a formal review from you?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] b2iscsi: Fix memory leak in mgmt_set_ip()
2014-07-01 18:53 ` Christoph Hellwig
@ 2014-07-07 11:29 ` Tomas Henzl
0 siblings, 0 replies; 6+ messages in thread
From: Tomas Henzl @ 2014-07-07 11:29 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Maurizio Lombardi, jayamohan.kallickal, linux-scsi, michaelc,
JBottomley
On 07/01/2014 08:53 PM, Christoph Hellwig wrote:
> On Sun, Jun 29, 2014 at 08:28:34PM +0200, Tomas Henzl wrote:
>> On 06/27/2014 09:38 PM, Maurizio Lombardi wrote:
>>>>> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
>>>>> index 07934b0..a3e5648 100644
>>>>> --- a/drivers/scsi/be2iscsi/be_mgmt.c
>>>>> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
>>>>> @@ -1015,7 +1015,7 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
>>>>> if (if_info->dhcp_state) {
>>>>> beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
>>>>> "BG_%d : DHCP Already Enabled\n");
>>>>> - return 0;
>>>>> + goto exit;
>>>> rc should be initialised - so this needs to be added to your patch
>>> It is initialized at the beginning of the function, and it must contain zero when the "goto exit" is executed.
>> Correct, I haven't noticed the initialisation, sorry for the confusion.
> Is this a formal review from you?
I was under time pressure so I just wanted to remove my previous negative comment, but now -
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-07 11:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-27 12:55 [PATCH] b2iscsi: Fix memory leak in mgmt_set_ip() Maurizio Lombardi
2014-06-27 16:32 ` Tomas Henzl
2014-06-27 19:38 ` Maurizio Lombardi
2014-06-29 18:28 ` Tomas Henzl
2014-07-01 18:53 ` Christoph Hellwig
2014-07-07 11:29 ` Tomas Henzl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox