diff mbox series

tomoyo: Use error code from kern_path() rather than -ENOENT.

Message ID 1564659355-12826-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive)
State New, archived
Headers show
Series tomoyo: Use error code from kern_path() rather than -ENOENT. | expand

Commit Message

Tetsuo Handa Aug. 1, 2019, 11:35 a.m. UTC
Takeshi Misawa has pointed out that tomoyo_find_next_domain() is returning
-ENOENT when tomoyo_realpath_nofollow() failed [1]. That error code was
chosen based on an assumption that when tomoyo_realpath_nofollow() fails,
the cause of failure is kern_path() failure due to a race window that
the pathname used for do_open_execat() from __do_execve_file() was removed
before tomoyo_find_next_domain() is called.

Since tomoyo_realpath_nofollow() is called by tomoyo_find_next_domain()
only, and __do_execve_file() makes sure that bprm->filename != NULL, let's
inline tomoyo_realpath_nofollow() into tomoyo_find_next_domain().

It seems that tomoyo_realpath_nofollow() is currently broken by
commit 449325b52b7a6208 ("umh: introduce fork_usermode_blob() helper")
when do_execve_file() is used. To fix it, we will need to know whether
do_open_execat() was called before tomoyo_find_next_domain() is called.
To fix it in a more accurate and race-free way, we will need to calculate
both LOOKUP_FOLLOW pathname and !LOOKUP_FOLLOW pathname at the same time.

[1] https://lkml.kernel.org/r/20190801030323.GA1958@DESKTOP

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reported-by: Takeshi Misawa <jeliantsurux@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Alexei Starovoitov <ast@kernel.org>
---
 security/tomoyo/common.h   |  1 -
 security/tomoyo/domain.c   | 18 ++++++++++++++----
 security/tomoyo/realpath.c | 20 --------------------
 3 files changed, 14 insertions(+), 25 deletions(-)
diff mbox series

Patch

diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
index 050473df5809..58b51a21cf9c 100644
--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -957,7 +957,6 @@  char *tomoyo_init_log(struct tomoyo_request_info *r, int len, const char *fmt,
 		      va_list args);
 char *tomoyo_read_token(struct tomoyo_acl_param *param);
 char *tomoyo_realpath_from_path(const struct path *path);
-char *tomoyo_realpath_nofollow(const char *pathname);
 const char *tomoyo_get_exe(void);
 const char *tomoyo_yesno(const unsigned int value);
 const struct tomoyo_path_info *tomoyo_compare_name_union
diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c
index 8526a0a74023..5cd06bfd46c7 100644
--- a/security/tomoyo/domain.c
+++ b/security/tomoyo/domain.c
@@ -721,10 +721,20 @@  int tomoyo_find_next_domain(struct linux_binprm *bprm)
 	ee->r.obj = &ee->obj;
 	ee->obj.path1 = bprm->file->f_path;
 	/* Get symlink's pathname of program. */
-	retval = -ENOENT;
-	exename.name = tomoyo_realpath_nofollow(original_name);
-	if (!exename.name)
-		goto out;
+	{
+		struct path path;
+		int ret = kern_path(original_name, 0, &path);
+
+		if (ret) {
+			retval = ret;
+			exename.name = NULL;
+			goto out;
+		}
+		exename.name = tomoyo_realpath_from_path(&path);
+		path_put(&path);
+		if (!exename.name) /* retval was initialized with -ENONEM */
+			goto out;
+	}
 	tomoyo_fill_path_info(&exename);
 retry:
 	/* Check 'aggregator' directive. */
diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
index e7832448d721..70d456348e1c 100644
--- a/security/tomoyo/realpath.c
+++ b/security/tomoyo/realpath.c
@@ -321,23 +321,3 @@  char *tomoyo_realpath_from_path(const struct path *path)
 		tomoyo_warn_oom(__func__);
 	return name;
 }
-
-/**
- * tomoyo_realpath_nofollow - Get realpath of a pathname.
- *
- * @pathname: The pathname to solve.
- *
- * Returns the realpath of @pathname on success, NULL otherwise.
- */
-char *tomoyo_realpath_nofollow(const char *pathname)
-{
-	struct path path;
-
-	if (pathname && kern_path(pathname, 0, &path) == 0) {
-		char *buf = tomoyo_realpath_from_path(&path);
-
-		path_put(&path);
-		return buf;
-	}
-	return NULL;
-}