diff mbox

[5/6] kpartx: fix device checks

Message ID 1494349025-6561-6-git-send-email-bmarzins@redhat.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show

Commit Message

Benjamin Marzinski May 9, 2017, 4:57 p.m. UTC
There are a number of issues in the the kpartx device checking code.
First, it accepts files that are neither regular files or a block device
nodes (you can run kpartx on character devices or directories, and it
will treat them as block devices). When trying to figure out the
basename of a device, the code returns garbage if the path doesn't
include a '/'. Finally, the set_delimiter code can access memory outside
of the string if an empty string is passed in.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 kpartx/kpartx.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

Comments

Martin Wilck May 11, 2017, 8:30 p.m. UTC | #1
On Tue, 2017-05-09 at 11:57 -0500, Benjamin Marzinski wrote:
> There are a number of issues in the the kpartx device checking code.
> First, it accepts files that are neither regular files or a block
> device
> nodes (you can run kpartx on character devices or directories, and it
> will treat them as block devices). When trying to figure out the
> basename of a device, the code returns garbage if the path doesn't
> include a '/'. Finally, the set_delimiter code can access memory
> outside
> of the string if an empty string is passed in.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>
diff mbox

Patch

diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index cc7e2e7..a1af156 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -135,10 +135,13 @@  set_delimiter (char * device, char * delimiter)
 {
 	char * p = device;
 
-	while (*(p++) != 0x0)
+	if (*p == 0x0)
+		return;
+
+	while (*(++p) != 0x0)
 		continue;
 
-	if (isdigit(*(p - 2)))
+	if (isdigit(*(p - 1)))
 		*delimiter = 'p';
 }
 
@@ -157,15 +160,17 @@  strip_slash (char * device)
 static int
 find_devname_offset (char * device)
 {
-	char *p, *q = NULL;
+	char *p, *q;
 
-	p = device;
+	q = p = device;
 
-	while (*p++)
+	while (*p) {
 		if (*p == '/')
-			q = p;
+			q = p + 1;
+		p++;
+	}
 
-	return (int)(q - device) + 1;
+	return (int)(q - device);
 }
 
 static char *
@@ -381,6 +386,10 @@  main(int argc, char **argv){
 			exit (1);
 		}
 	}
+	else if (!S_ISBLK(buf.st_mode)) {
+		fprintf(stderr, "invalid device: %s\n", device);
+		exit(1);
+	}
 
 	off = find_devname_offset(device);