diff mbox series

[RESEND] nbd: code clean for nbd_genl_status()

Message ID 1636000703-13217-1-git-send-email-wubo40@huawei.com (mailing list archive)
State New, archived
Headers show
Series [RESEND] nbd: code clean for nbd_genl_status() | expand

Commit Message

Wu Bo Nov. 4, 2021, 4:38 a.m. UTC 
A simple code clean for nbd_genl_status()

Signed-off-by: Wu Bo <wubo40@huawei.com>
---
 drivers/block/nbd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Christoph Hellwig Nov. 4, 2021, 8:48 a.m. UTC | #1 
On Thu, Nov 04, 2021 at 12:38:23PM +0800, Wu Bo wrote:
>  	if (!reply_head) {
> -		nlmsg_free(reply);
>  		goto out;
>  	}

Please also drop the now pointless braces.

>  out:
> +	if (reply)
> +		nlmsg_free(reply);

Please just use a different label for just unlocking vs also freeing
the reply.
Wu Bo Nov. 5, 2021, 1:46 a.m. UTC | #2 
On 2021/11/4 16:48, Christoph Hellwig wrote:
> On Thu, Nov 04, 2021 at 12:38:23PM +0800, Wu Bo wrote:
>>   	if (!reply_head) {
>> -		nlmsg_free(reply);
>>   		goto out;
>>   	}
> 
> Please also drop the now pointless braces.
> 
ok. Will do in v2.
>>   out:
>> +	if (reply)
>> +		nlmsg_free(reply);
> 
> Please just use a different label for just unlocking vs also freeing
> the reply.
> .
ok. Will do in v2.

Thanks.

Wu Bo
Josef Bacik Nov. 5, 2021, 10:01 a.m. UTC | #3 
On Thu, Nov 04, 2021 at 12:38:23PM +0800, Wu Bo wrote:
> A simple code clean for nbd_genl_status()
> 
> Signed-off-by: Wu Bo <wubo40@huawei.com>

This isn't ok, genlmsg_reply() free's the reply, this introduces a double free.
Thanks,

Josef
diff mbox series

Patch

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index b47b2a8..175ea97 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -2375,7 +2375,6 @@  static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
 	reply_head = genlmsg_put_reply(reply, info, &nbd_genl_family, 0,
 				       NBD_CMD_STATUS);
 	if (!reply_head) {
-		nlmsg_free(reply);
 		goto out;
 	}
 
@@ -2383,7 +2382,6 @@  static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
 	if (index == -1) {
 		ret = idr_for_each(&nbd_index_idr, &status_cb, reply);
 		if (ret) {
-			nlmsg_free(reply);
 			goto out;
 		}
 	} else {
@@ -2392,7 +2390,6 @@  static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
 		if (nbd) {
 			ret = populate_nbd_status(nbd, reply);
 			if (ret) {
-				nlmsg_free(reply);
 				goto out;
 			}
 		}
@@ -2401,6 +2398,8 @@  static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
 	genlmsg_end(reply, reply_head);
 	ret = genlmsg_reply(reply, info);
 out:
+	if (reply)
+		nlmsg_free(reply);
 	mutex_unlock(&nbd_index_mutex);
 	return ret;
 }