Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 3 Jun 2009 15:08:04 +1000
From:      Benno Rice <benno@jeamland.net>
To:        current@freebsd.org
Subject:   Re: CFR: sanity checking arguments to kldload(8)
Message-ID:  <F4AAD80A-CDF6-4FD0-82D8-A7FBE0BEB993@jeamland.net>
In-Reply-To: <B8E26963-4413-499B-A512-A5ECF997D11A@jeamland.net>
References:  <B8E26963-4413-499B-A512-A5ECF997D11A@jeamland.net>

next in thread | previous in thread | raw e-mail | index | archive | help

--Apple-Mail-5--589487971
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed;
	delsp=yes
Content-Transfer-Encoding: 7bit


On 03/06/2009, at 1:03 PM, Benno Rice wrote:

> The attached patch performs some sanity checking on arguments passed  
> to kldload(8).  Specifically, if an argument looks like a filename  
> but lacks a path (eg, 'xfs.ko' as opposed to 'xfs' or './xfs.ko') it  
> checks to see if a file with that name is in the current directory.   
> If it is, it checks the current module path to see if a file with  
> that name also exists there (possibly in an earlier entry if the  
> current directory is in the module path), if so it warns the user  
> that that module will be loaded and not the one in the current  
> directory.  If not, it tells the user how to use a path to load the  
> module.
>
> A -q option is added to quieten the output if desired.


I've had some feedback that the instructions on how to load the module  
from the current directory
should instead be in the manual page.  I've done this and added extra  
verbiage to point out the potential source of confusion wherein bare  
filenames (eg. foo.ko as opposed to ./foo.ko) will only ever be loaded  
from the module path.

Further comments appreciated.


--Apple-Mail-5--589487971
Content-Disposition: attachment;
	filename=kldload-2.diff
Content-Type: application/octet-stream; x-unix-mode=0644; name="kldload-2.diff"
Content-Transfer-Encoding: 7bit

Index: sbin/kldload/kldload.8
===================================================================
--- sbin/kldload/kldload.8	(revision 193292)
+++ sbin/kldload/kldload.8	(working copy)
@@ -50,10 +50,22 @@
 .Nm .
 It does not hurt to specify it though.
 .Pp
+If a bare filename is requested it will only be loaded if it is found within
+the module path as defined by the sysctl
+.Va kern.module_path .
+To load a module from the current directory it must be specified as a full or
+relative path.
+The
+.Nm
+utility will warn if a module is requested as a bare filename and is present
+in the current directory.
+.Pp
 The following option is available:
 .Bl -tag -width indent
 .It Fl v
 Be more verbose.
+.It Fl q
+Silence any extraneous warnings.
 .El
 .Sh FILES
 .Bl -tag -width /boot/kernel -compact
@@ -64,6 +76,26 @@
 .El
 .Sh EXIT STATUS
 .Ex -std
+.Sh EXAMPLES
+To load by module name:
+.Bd -literal -offset indent
+\*[Gt] kldload foo
+.Ed
+.Pp
+To load by file name within the module path:
+.Bd -literal -offset indent
+\*[Gt] kldload foo.ko
+.Ed
+.Pp
+To load by relative path:
+.Bd -literal -offset indent
+\*[Gt] kldload ./foo.ko
+.Ed
+.Pp
+To load by full path:
+.Bd -literal -offset indent
+\*[Gt] kldload /boot/kernel/foo.ko
+.Ed
 .Sh AUTOMATICALLY LOADING MODULES
 Some modules (pf, ipfw, ipf, etc.) may be automatically loaded at boot
 time when the corresponding
Index: sbin/kldload/kldload.c
===================================================================
--- sbin/kldload/kldload.c	(revision 193292)
+++ sbin/kldload/kldload.c	(working copy)
@@ -30,10 +30,99 @@
 #include <err.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
 #include <sys/param.h>
 #include <sys/linker.h>
+#include <sys/sysctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 
+#define	PATHCTL	"kern.module_path"
+
+/*
+ * Check to see if the requested module is specified as a filename with no
+ * path.  If so and if a file by the same name exists in the module path,
+ * warn the user that the module in the path will be used in preference.
+ */
+static int
+path_check(const char *kldname, int quiet)
+{
+	int	mib[5], found;
+	size_t	miblen, pathlen;
+	char	kldpath[MAXPATHLEN];
+	char	*path, *tmppath, *element;
+	struct	stat sb;
+	dev_t	dev;
+	ino_t	ino;
+
+	if (strchr(kldname, '/') != NULL) {
+		return (0);
+	}
+	if (strstr(kldname, ".ko") == NULL) {
+		return (0);
+	}
+	if (stat(kldname, &sb) != 0) {
+		return (0);
+	}
+
+	found = 0;
+	dev = sb.st_dev;
+	ino = sb.st_ino;
+
+	miblen = sizeof(mib) / sizeof(mib[0]);
+	if (sysctlnametomib(PATHCTL, mib, &miblen) != 0) {
+		err(1, "sysctlnametomib(%s)", PATHCTL);
+	}
+	if (sysctl(mib, miblen, NULL, &pathlen, NULL, 0) == -1) {
+		err(1, "getting path: sysctl(%s) - size only", PATHCTL);
+	}
+	path = malloc(pathlen + 1);
+	if (path == NULL) {
+		err(1, "allocating %lu bytes for the path",
+		    (unsigned long)pathlen + 1);
+	}
+	if (sysctl(mib, miblen, path, &pathlen, NULL, 0) == -1) {
+		err(1, "getting path: sysctl(%s)", PATHCTL);
+	}
+	tmppath = path;
+
+	while ((element = strsep(&tmppath, ";")) != NULL) {
+		strlcpy(kldpath, element, MAXPATHLEN);
+		if (kldpath[strlen(kldpath) - 1] != '/') {
+			strlcat(kldpath, "/", MAXPATHLEN);
+		}
+		strlcat(kldpath, kldname, MAXPATHLEN);
+				
+		if (stat(kldpath, &sb) == -1) {
+			continue;
+		}	
+
+		found = 1;
+
+		if (sb.st_dev != dev || sb.st_ino != ino) {
+			if (!quiet) {
+				warnx("%s will be loaded from %s, not the "
+				    "current directory", kldname, element);
+			}
+			break;
+		} else if (sb.st_dev == dev && sb.st_ino == ino) {
+			return (0);
+		}
+	}
+
+	free(path);
+	
+	if (!found) {
+		if (!quiet) {
+			warnx("%s is not in the module path", kldname);
+		}
+		return (-1);
+	}
+	
+	return (0);
+}
+
 static void
 usage(void)
 {
@@ -48,14 +137,21 @@
     int errors;
     int fileid;
     int verbose;
+    int quiet;
 
     errors = 0;
     verbose = 0;
-
-    while ((c = getopt(argc, argv, "v")) != -1)
+    quiet = 0;
+    
+    while ((c = getopt(argc, argv, "qv")) != -1)
 	switch (c) {
+	case 'q':
+	    quiet = 1;
+	    verbose = 0;
+	    break;
 	case 'v':
 	    verbose = 1;
+	    quiet = 0;
 	    break;
 	default:
 	    usage();
@@ -67,13 +163,17 @@
 	usage();
 
     while (argc-- != 0) {
-	fileid = kldload(argv[0]);
-	if (fileid < 0) {
-	    warn("can't load %s", argv[0]);
-	    errors++;
-	} else
-	    if (verbose)
-		printf("Loaded %s, id=%d\n", argv[0], fileid);
+	if (path_check(argv[0], quiet) == 0) {
+		fileid = kldload(argv[0]);
+		if (fileid < 0) {
+		    warn("can't load %s", argv[0]);
+		    errors++;
+		} else
+		    if (verbose)
+			printf("Loaded %s, id=%d\n", argv[0], fileid);
+	} else {
+		errors++;
+	}
 	argv++;
     }
 

--Apple-Mail-5--589487971
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed
Content-Transfer-Encoding: 7bit




-- 
Benno Rice
benno@jeamland.net




--Apple-Mail-5--589487971--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F4AAD80A-CDF6-4FD0-82D8-A7FBE0BEB993>