Factor out checks for mounted root file system

[• hselasky]

Summary

PR: 241639
MFC after: 1 week
Sponsored by: Mellanox Technologies

[• hselasky]

sys/compat/linuxkpi/common/include/linux/kmod.h

46 ↗

(On Diff #64938)

Should you check if p_fd != NULL ?

[• hselasky]

I wonder if this check should be factored out? It is not only the LinuxKPI which access these code paths too early, but USB drivers loading firmware .ko's can also do the same??

[• hselasky]

Index: sys/kern/kern_linker.c
===================================================================
--- sys/kern/kern_linker.c	(revision 355108)
+++ sys/kern/kern_linker.c	(working copy)
@@ -2067,6 +2067,7 @@
     struct linker_file *parent, const struct mod_depend *verinfo,
     struct linker_file **lfpp)
 {
+	struct proc *cp = curproc;
 	linker_file_t lfdep;
 	const char *filename;
 	char *pathname;
@@ -2079,13 +2080,13 @@
  		 */
 		KASSERT(verinfo == NULL, ("linker_load_module: verinfo"
 		    " is not NULL"));
-		if (rootvnode == NULL)
+		if (rootvnode == NULL || cp->p_fd == NULL || cp->p_fd->fd_rdir == NULL)
 			return (ENXIO);
 		pathname = linker_search_kld(kldname);
 	} else {
 		if (modlist_lookup2(modname, verinfo) != NULL)
 			return (EEXIST);
-		if (rootvnode == NULL)
+		if (rootvnode == NULL || cp->p_fd == NULL || cp->p_fd->fd_rdir == NULL)
 			return (ENXIO);
 		if (kldname != NULL)
 			pathname = strdup(kldname, M_LINKER);

Why not like this? Or remove the rootvnode == NULL and only test fd_rdir ?

--HPS

[kib]

In D22571#493584, @hselasky wrote:

Index: sys/kern/kern_linker.c
===================================================================
--- sys/kern/kern_linker.c	(revision 355108)
+++ sys/kern/kern_linker.c	(working copy)
@@ -2067,6 +2067,7 @@
     struct linker_file *parent, const struct mod_depend *verinfo,
     struct linker_file **lfpp)
 {
+	struct proc *cp = curproc;
 	linker_file_t lfdep;
 	const char *filename;
 	char *pathname;
@@ -2079,13 +2080,13 @@
  		 */
 		KASSERT(verinfo == NULL, ("linker_load_module: verinfo"
 		    " is not NULL"));
-		if (rootvnode == NULL)
+		if (rootvnode == NULL || cp->p_fd == NULL || cp->p_fd->fd_rdir == NULL)
 			return (ENXIO);
 		pathname = linker_search_kld(kldname);
 	} else {
 		if (modlist_lookup2(modname, verinfo) != NULL)
 			return (EEXIST);
-		if (rootvnode == NULL)
+		if (rootvnode == NULL || cp->p_fd == NULL || cp->p_fd->fd_rdir == NULL)
 			return (ENXIO);
 		if (kldname != NULL)
 			pathname = strdup(kldname, M_LINKER);

Why not like this? Or remove the rootvnode == NULL and only test fd_rdir ?

Check for p_fd == NULL is really not needed, because we initalize it even for proc0 with valid file descriptor table.

I think that keeping rootvnode check is still needed/useful.

[• hselasky]

Keep the rootvnode check in case the root file system gets unmounted.
Update comment.

[kib]

sys/kern/kern_linker.c

2070 ↗

(On Diff #64941)

No initialization in declaration. We usually name the single variable in function designated a process pointer 'p'.

2084 ↗

(On Diff #64941)

p_fd == NULL check is not needed.

[• hselasky]

Handle comments from @kib

[kib]

In fact, I do not see much need in introducing 'p': you reference curproc only once in each branch, so caching the pointer does not save anything.