public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: John Johansen <john.johansen@canonical.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Subject: Re: [PATCH 01/12] Miscellaneous functions and defines needed by AppArmor, including the base path resolution routines.
Date: Sat, 20 Feb 2010 04:24:25 -0800	[thread overview]
Message-ID: <4B7FD479.5020807@canonical.com> (raw)
In-Reply-To: <20100219110320.GL30031@ZenIV.linux.org.uk>

Al Viro wrote:
> On Fri, Feb 19, 2010 at 01:36:17AM -0800, john.johansen@canonical.com wrote:
> 
>> +	spin_lock(&dcache_lock);
>> +	/* There is a race window between path lookup here and the
>> +	 * need to strip the " (deleted) string that __d_path applies
>> +	 * Detect the race and relookup the path
>> +	 *
>> +	 * The stripping of (deleted) is a hack that could be removed
>> +	 * with an updated __d_path
> 
> Yes, it could.  Where's the patch doing just that?  Or discussion of
> desired interface, at lease...
> 
Glad you asked I was going to include it with a couple other patches to
__d_path.  Basically trying to separate proposed changes to __d_path
from AppArmor, as most people who will be looking at the __d_path code
would rather not have the whole AppArmor patchset dropped on them.

The attached patch is just a first pass and a starting point for
discussion.  I only modified TOMOYO to match current behavior of the
kernel and will leave it to Tetsuo to modify TOMOYO to make use
of the flag.

---

Make __d_path unambiguous for deleted files

__d_path currently appends the string " (deleted)" to deleted entries, but
this results in an ambiguous path.  This is problematic for TOMOYO and
AppArmor as they use __d_path to retrieve path names for file objects.

This patch matches the appending the " (deleted)" string optional,
removing the need for AppArmor and TOMOYO to remove the string after
the fact.

Signed-off-by: John Johansen <john.johansen@canonical.com>

diff --git a/fs/dcache.c b/fs/dcache.c
index df49666..44c2afc 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1890,9 +1890,10 @@ static int prepend_name(char **buffer, int *buflen, struct qstr *name)
  * @root: root vfsmnt/dentry (may be modified by this function)
  * @buffer: buffer to return value in
  * @buflen: buffer length
- *
+ * @mark_deleted: for deleted entries determine whether to append " (deleted)"
  * Convert a dentry into an ASCII path name. If the entry has been deleted
- * the string " (deleted)" is appended. Note that this is ambiguous.
+ * and @mark_deleted is true then the string " (deleted)" is appended.
+ * Note that this is ambiguous.
  *
  * Returns a pointer into the buffer or an error code if the
  * path was too long.
@@ -1903,7 +1904,7 @@ static int prepend_name(char **buffer, int *buflen, struct qstr *name)
  * root is changed (without modifying refcounts).
  */
 char *__d_path(const struct path *path, struct path *root,
-	       char *buffer, int buflen)
+	       char *buffer, int buflen, bool mark_deleted)
 {
 	struct dentry *dentry = path->dentry;
 	struct vfsmount *vfsmnt = path->mnt;
@@ -1912,7 +1913,7 @@ char *__d_path(const struct path *path, struct path *root,
 
 	spin_lock(&vfsmount_lock);
 	prepend(&end, &buflen, "\0", 1);
-	if (d_unlinked(dentry) &&
+	if (d_unlinked(dentry) && mark_deleted &&
 		(prepend(&end, &buflen, " (deleted)", 10) != 0))
 			goto Elong;
 
@@ -2019,7 +2020,7 @@ char *d_path(const struct path *path, char *buf, int buflen)
 	read_unlock(&current->fs->lock);
 	spin_lock(&dcache_lock);
 	tmp = root;
-	res = __d_path(path, &tmp, buf, buflen);
+	res = __d_path(path, &tmp, buf, buflen, DPATH_MARK_DELETED);
 	spin_unlock(&dcache_lock);
 	path_put(&root);
 	return res;
@@ -2124,7 +2125,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size)
 		struct path tmp = root;
 		char * cwd;
 
-		cwd = __d_path(&pwd, &tmp, page, PAGE_SIZE);
+		cwd = __d_path(&pwd, &tmp, page, PAGE_SIZE, DPATH_MARK_DELETED);
 		spin_unlock(&dcache_lock);
 
 		error = PTR_ERR(cwd);
diff --git a/fs/seq_file.c b/fs/seq_file.c
index eae7d9d..f046bd3 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -463,7 +463,7 @@ int seq_path_root(struct seq_file *m, struct path *path, struct path *root,
 		char *p;
 
 		spin_lock(&dcache_lock);
-		p = __d_path(path, root, buf, size);
+		p = __d_path(path, root, buf, size, DPATH_MARK_DELETED);
 		spin_unlock(&dcache_lock);
 		res = PTR_ERR(p);
 		if (!IS_ERR(p)) {
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 30b93b2..021f515 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -309,9 +309,12 @@ extern int d_validate(struct dentry *, struct dentry *);
 /*
  * helper function for dentry_operations.d_dname() members
  */
+#define DPATH_MARK_DELETED 1
+
 extern char *dynamic_dname(struct dentry *, char *, int, const char *, ...);
 
-extern char *__d_path(const struct path *path, struct path *root, char *, int);
+extern char *__d_path(const struct path *path, struct path *root, char *, int,
+		      bool);
 extern char *d_path(const struct path *, char *, int);
 extern char *dentry_path(struct dentry *, char *, int);
 
diff --git a/security/tomoyo/realpath.c b/security/tomoyo/realpath.c
index 18369d4..0f4d4e2 100644
--- a/security/tomoyo/realpath.c
+++ b/security/tomoyo/realpath.c
@@ -106,7 +106,8 @@ int tomoyo_realpath_from_path2(struct path *path, char *newname,
 		spin_unlock(&vfsmount_lock);
 		spin_lock(&dcache_lock);
 		tmp = ns_root;
-		sp = __d_path(path, &tmp, newname, newname_len);
+		sp = __d_path(path, &tmp, newname, newname_len,
+			      DPATH_MARK_DELETED);
 		spin_unlock(&dcache_lock);
 		path_put(&root);
 		path_put(&ns_root);

  parent reply	other threads:[~2010-02-20 12:24 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-19  9:36 [AppArmor #4 0/12] AppArmor security module john.johansen
2010-02-19  9:36 ` [PATCH 01/12] Miscellaneous functions and defines needed by AppArmor, including the base path resolution routines john.johansen
2010-02-19 11:03   ` Al Viro
2010-02-20 12:17     ` John Johansen
2010-02-20 17:25       ` John Johansen
2010-02-20 19:10         ` John Johansen
2010-02-20 12:24     ` John Johansen [this message]
2010-02-19  9:36 ` [PATCH 02/12] Update kenel audit range comments to show AppArmor's registered range of 1500-1599. This range used to be reserved for LSPP but LSPP uses the SELinux range and the range was given to AppArmor. Patch is not in mainline -- pending AppArmor code submission to lkml john.johansen
2010-02-19  9:36 ` [PATCH 03/12] AppArmor contexts attach profiles and state to tasks, files, etc. when a direct profile reference is not sufficient john.johansen
2010-02-19  9:36 ` [PATCH 04/12] The basic routines and defines for AppArmor policy. AppArmor policy is defined by a few basic components. profiles - the basic unit of confinement contain all the information to enforce policy on a task john.johansen
2010-02-19  9:36 ` [PATCH 05/12] A basic dfa matching engine based off the dfa engine in the Dragon Book. It uses simple row comb compression with a check field john.johansen
2010-02-19  9:36 ` [PATCH 06/12] AppArmor policy is loaded in a platform independent flattened binary stream. Verify and unpack the data converting it to the internal format needed for enforcement john.johansen
2010-02-19  9:36 ` [PATCH 07/12] AppArmor /proc/<pid>/attr/* and apparmorfs interfaces to userspace john.johansen
2010-02-19  9:36 ` [PATCH 08/12] AppArmor: file enforcement routines john.johansen
2010-02-19  9:36 ` [PATCH 09/12] AppArmor ipc, rlimit, network and capability routines john.johansen
2010-02-19  9:36 ` [PATCH 10/12] AppArmor routines for controlling domain transitions john.johansen
2010-02-19  9:36 ` [PATCH 11/12] AppArmor hooks to interface with the LSM, module parameters and initialization john.johansen
2010-02-22 22:14   ` Serge E. Hallyn
2010-02-23  7:58     ` John Johansen
2010-02-19  9:36 ` [PATCH 12/12] Kconfig and Makefiles to enable configuration and building of AppArmor john.johansen
2010-02-22 22:16   ` Serge E. Hallyn
2010-02-23  7:45     ` John Johansen
2010-03-03  7:50       ` Kees Cook
2010-02-23  1:59 ` [AppArmor #4 0/12] AppArmor security module Tetsuo Handa
2010-02-23  8:38   ` John Johansen
2010-02-23  8:31 ` Tetsuo Handa
2010-02-23  9:17   ` John Johansen
2010-02-26  3:22 ` Tetsuo Handa
2010-02-26  6:31 ` Tetsuo Handa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B7FD479.5020807@canonical.com \
    --to=john.johansen@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox