linux-kernel - [PATCH 1/2] Drivers: hv: vmbus: Remove duplication and cleanup code in create_gpadl

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-next>] [day] [month] [year] [list]

Date: Thu, 11 Jan 2024 08:54:50 -0800
From: mhkelley58@...il.com
To: haiyangz@...rosoft.com,
	wei.liu@...nel.org,
	decui@...rosoft.com,
	dan.carpenter@...aro.org,
	Markus.Elfring@....de,
	linux-kernel@...r.kernel.org,
	linux-hyperv@...r.kernel.org
Subject: [PATCH 1/2] Drivers: hv: vmbus: Remove duplication and cleanup code in create_gpadl_header()

From: Michael Kelley <mhklinux@...look.com>

create_gpadl_header() creates a message header, and one or more message
bodies if the number of GPADL entries exceeds what fits in the
header. Currently the code for creating the message header is
duplicated in the two halves of the main "if" statement governing
whether message bodies are created.

Eliminate the duplication by making minor tweaks to the logic and
associated comments. While here, simplify the handling of memory
allocation errors, and use umin() instead of open coding it.

For ease of review, the indentation of sizable chunks of code is
*not* changed.  A follow-on patch updates only the indentation.

No functional change.

Suggested-by: Dan Carpenter <dan.carpenter@...aro.org>
Signed-off-by: Michael Kelley <mhklinux@...look.com>
---

Because the GPADL messages interact with Hyper-V and use
PFNs, I tested on x86, ARM64 with 4K page size, and ARM64
with 64K page size.  All look good.


 drivers/hv/channel.c | 54 ++++++++------------------------------------
 1 file changed, 10 insertions(+), 44 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 56f7e06c673e..604f5aff8502 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -322,21 +322,17 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
 
 	pagecount = hv_gpadl_size(type, size) >> HV_HYP_PAGE_SHIFT;
 
-	/* do we need a gpadl body msg */
 	pfnsize = MAX_SIZE_CHANNEL_MESSAGE -
 		  sizeof(struct vmbus_channel_gpadl_header) -
 		  sizeof(struct gpa_range);
-	pfncount = pfnsize / sizeof(u64);
+	pfncount = umin(pagecount, pfnsize / sizeof(u64));
 
-	if (pagecount > pfncount) {
-		/* we need a gpadl body */
-		/* fill in the header */
 		msgsize = sizeof(struct vmbus_channel_msginfo) +
 			  sizeof(struct vmbus_channel_gpadl_header) +
 			  sizeof(struct gpa_range) + pfncount * sizeof(u64);
 		msgheader =  kzalloc(msgsize, GFP_KERNEL);
 		if (!msgheader)
-			goto nomem;
+			return -ENOMEM;
 
 		INIT_LIST_HEAD(&msgheader->submsglist);
 		msgheader->msgsize = msgsize;
@@ -356,18 +352,17 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
 		pfnsum = pfncount;
 		pfnleft = pagecount - pfncount;
 
-		/* how many pfns can we fit */
+		/* how many pfns can we fit in a body message */
 		pfnsize = MAX_SIZE_CHANNEL_MESSAGE -
 			  sizeof(struct vmbus_channel_gpadl_body);
 		pfncount = pfnsize / sizeof(u64);
 
-		/* fill in the body */
+		/*
+		 * If pfnleft is zero, everything fits in the header and no body
+		 * messages are needed
+		 */
 		while (pfnleft) {
-			if (pfnleft > pfncount)
-				pfncurr = pfncount;
-			else
-				pfncurr = pfnleft;
-
+			pfncurr = umin(pfncount, pfnleft);
 			msgsize = sizeof(struct vmbus_channel_msginfo) +
 				  sizeof(struct vmbus_channel_gpadl_body) +
 				  pfncurr * sizeof(u64);
@@ -386,8 +381,8 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
 					list_del(&pos->msglistentry);
 					kfree(pos);
 				}
-
-				goto nomem;
+				kfree(msgheader);
+				return -ENOMEM;
 			}
 
 			msgbody->msgsize = msgsize;
@@ -410,37 +405,8 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
 			pfnsum += pfncurr;
 			pfnleft -= pfncurr;
 		}
-	} else {
-		/* everything fits in a header */
-		msgsize = sizeof(struct vmbus_channel_msginfo) +
-			  sizeof(struct vmbus_channel_gpadl_header) +
-			  sizeof(struct gpa_range) + pagecount * sizeof(u64);
-		msgheader = kzalloc(msgsize, GFP_KERNEL);
-		if (msgheader == NULL)
-			goto nomem;
-
-		INIT_LIST_HEAD(&msgheader->submsglist);
-		msgheader->msgsize = msgsize;
-
-		gpadl_header = (struct vmbus_channel_gpadl_header *)
-			msgheader->msg;
-		gpadl_header->rangecount = 1;
-		gpadl_header->range_buflen = sizeof(struct gpa_range) +
-					 pagecount * sizeof(u64);
-		gpadl_header->range[0].byte_offset = 0;
-		gpadl_header->range[0].byte_count = hv_gpadl_size(type, size);
-		for (i = 0; i < pagecount; i++)
-			gpadl_header->range[0].pfn_array[i] = hv_gpadl_hvpfn(
-				type, kbuffer, size, send_offset, i);
-
-		*msginfo = msgheader;
-	}
 
 	return 0;
-nomem:
-	kfree(msgheader);
-	kfree(msgbody);
-	return -ENOMEM;
 }
 
 /*
-- 
2.25.1