fix a leak in replace_fd() users

replace_fd() began with "eats a reference, tries to insert into descriptor table" semantics; at some point I'd switched it to much saner current behaviour ("try to insert into descriptor table, grabbing a new reference if inserted; caller should do fput() in any case"), but forgot to update the callers. Mea culpa... [Spotted by Pavel Roskin, who has really weird system with pipe-fed coredumps as part of what he considers a normal boot ;-)] Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

fix a leak in replace_fd() users
replace_fd() began with "eats a reference, tries to insert into descriptor table" semantics; at some point I'd switched it to much saner current behaviour ("try to insert into descriptor table, grabbing a new reference if inserted; caller should do fput() in any case"), but forgot to update the callers. Mea culpa... [Spotted by Pavel Roskin, who has really weird system with pipe-fed coredumps as part of what he considers a normal boot ;-)] Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Al Viro
1 parent dd8e8c4a2c
Showing 2 changed files with 10 additions and 13 deletions Side-by-side Diff
fs/coredump.c
security/selinux/hooks.c
@@ -450,11 +450,12 @@
  
 	cp->file = files[1];
  
-	replace_fd(0, files[0], 0);
+	err = replace_fd(0, files[0], 0);
+	fput(files[0]);
 	/* and disallow core files too */
 	current->signal->rlim[RLIMIT_CORE] = (struct rlimit){1, 1};
  
-	return 0;
+	return err;
 }
  
 void do_coredump(siginfo_t *siginfo, struct pt_regs *regs)
@@ -2132,18 +2132,14 @@
 		return;
  
 	devnull = dentry_open(&selinux_null, O_RDWR, cred);
-	if (!IS_ERR(devnull)) {
-		/* replace all the matching ones with this */
-		do {
-			replace_fd(n - 1, get_file(devnull), 0);
-		} while ((n = iterate_fd(files, n, match_file, cred)) != 0);
+	if (IS_ERR(devnull))
+		devnull = NULL;
+	/* replace all the matching ones with this */
+	do {
+		replace_fd(n - 1, devnull, 0);
+	} while ((n = iterate_fd(files, n, match_file, cred)) != 0);
+	if (devnull)
 		fput(devnull);
-	} else {
-		/* just close all the matching ones */
-		do {
-			replace_fd(n - 1, NULL, 0);
-		} while ((n = iterate_fd(files, n, match_file, cred)) != 0);
-	}
 }
  
 /*
...	...	@@ -450,11 +450,12 @@
450	450
451	451	cp->file = files[1];
452	452
453		- replace_fd(0, files[0], 0);
	453	+ err = replace_fd(0, files[0], 0);
	454	+ fput(files[0]);
454	455	/* and disallow core files too */
455	456	current->signal->rlim[RLIMIT_CORE] = (struct rlimit){1, 1};
456	457
457		- return 0;
	458	+ return err;
458	459	}
459	460
460	461	void do_coredump(siginfo_t siginfo, struct pt_regs regs)
...	...	@@ -2132,18 +2132,14 @@
2132	2132	return;
2133	2133
2134	2134	devnull = dentry_open(&selinux_null, O_RDWR, cred);
2135		- if (!IS_ERR(devnull)) {
2136		- /* replace all the matching ones with this */
2137		- do {
2138		- replace_fd(n - 1, get_file(devnull), 0);
2139		- } while ((n = iterate_fd(files, n, match_file, cred)) != 0);
	2135	+ if (IS_ERR(devnull))
	2136	+ devnull = NULL;
	2137	+ /* replace all the matching ones with this */
	2138	+ do {
	2139	+ replace_fd(n - 1, devnull, 0);
	2140	+ } while ((n = iterate_fd(files, n, match_file, cred)) != 0);
	2141	+ if (devnull)
2140	2142	fput(devnull);
2141		- } else {
2142		- /* just close all the matching ones */
2143		- do {
2144		- replace_fd(n - 1, NULL, 0);
2145		- } while ((n = iterate_fd(files, n, match_file, cred)) != 0);
2146		- }
2147	2143	}
2148	2144
2149	2145	/*