Unreleased vnode in linux_sys_uselib()

[Maxime Villard]

Hi,
there seems to be a vnode issue in compat/linux/common/linux_uselib.c:

115 if (LINUX_N_MACHTYPE(&hdr) != LINUX_MID_MACHINE)
116 return ENOEXEC;

Here 'vp' is not released. Patch:

Index: linux_uselib.c
===================================================================
RCS file: /cvsroot/src/sys/compat/linux/common/linux_uselib.c,v
retrieving revision 1.30
diff -u -r1.30 linux_uselib.c
--- linux_uselib.c 28 Aug 2009 01:39:03 -0000 1.30
+++ linux_uselib.c 18 Oct 2014 10:48:53 -0000
@@ -103,17 +103,18 @@
if ((error = vn_rdwr(UIO_READ, vp, (void *) &hdr, LINUX_AOUT_HDR_SIZE,
0, UIO_SYSSPACE, IO_NODELOCKED, l->l_cred,
&rem, NULL))) {
- vrele(vp);
- return error;
+ goto out;
}

if (rem != 0) {
- vrele(vp);
- return ENOEXEC;
+ error = ENOEXEC;
+ goto out;
}

- if (LINUX_N_MACHTYPE(&hdr) != LINUX_MID_MACHINE)
- return ENOEXEC;
+ if (LINUX_N_MACHTYPE(&hdr) != LINUX_MID_MACHINE) {
+ error = ENOEXEC;
+ goto out;
+ }

magic = LINUX_N_MAGIC(&hdr);
taddr = hdr.a_entry & (~(PAGE_SIZE - 1));
@@ -123,7 +124,7 @@

error = vn_marktext(vp);
if (error)
- return (error);
+ goto out;

vcset.evs_cnt = 0;
vcset.evs_used = 0;
@@ -150,7 +151,7 @@

kill_vmcmds(&vcset);

+out:
vrele(vp);
-
return error;
}


And I'm wondering: what is the impact of such bugs? njoly@ tested a
small sample I sent him and apparently there doesn't seem to be any
glaring issue.

He also suggested to reject non-regular files, which makes sense since
this function aims at loading libraries:

Index: linux_uselib.c
===================================================================
RCS file: /cvsroot/src/sys/compat/linux/common/linux_uselib.c,v
retrieving revision 1.30
diff -u -p -r1.30 linux_uselib.c
--- linux_uselib.c 28 Aug 2009 01:39:03 -0000 1.30
+++ linux_uselib.c 18 Oct 2014 09:26:10 -0000
@@ -100,6 +100,11 @@ linux_sys_uselib(struct lwp *l,const st
if (error != 0)
return error;

+ if (vp->v_type != VREG) {
+ vrele(vp);
+ return EINVAL;
+ }
+
if ((error = vn_rdwr(UIO_READ,vp,(void *) &hdr,LINUX_AOUT_HDR_SIZE,
0,UIO_SYSSPACE,IO_NODELOCKED,l->l_cred,
&rem,NULL))) {

Found by my code scanner, with the help of ***@.

I would like one or two okayz before committing both.

[Nicolas Joly]

Post by Maxime Villard
Hi,
115 if (LINUX_N_MACHTYPE(&hdr) != LINUX_MID_MACHINE)
116 return ENOEXEC;

[...]

Post by Maxime Villard
small sample I sent him and apparently there doesn't seem to be any
glaring issue.

Got it. It prevents unmounting the corresponding filesystem ...

Without the vrele calls the vnode v_usecount value won't be lowered as
expected and kernel will wrongly think it's still in use at umount

I checked that your patch do fix this issue.

[Mindaugas Rasiukevicius]

Post by Maxime Villard
Hi,
<...>
small sample I sent him and apparently there doesn't seem to be any
glaring issue.

It is a resource leak; the vnode would never get freed.

The patch looks fine.