| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87bokugysl.fsf@xmission.com>
Date: Thu, 07 Jun 2012 22:25:46 -0700
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Al Viro <viro@...IV.linux.org.uk>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Dave Jones <davej@...hat.com>,
Linux Kernel <linux-kernel@...r.kernel.org>,
Miklos Szeredi <mszeredi@...e.cz>, Jan Kara <jack@...e.cz>,
Peter Zijlstra <peterz@...radead.org>,
linux-fsdevel@...r.kernel.org,
"J. Bruce Fields" <bfields@...hat.com>,
Sage Weil <sage@...dream.net>
Subject: Re: processes hung after sys_renameat, and 'missing' processes
Al Viro <viro@...IV.linux.org.uk> writes:
> On Fri, Jun 08, 2012 at 01:36:04AM +0100, Al Viro wrote:
>> Eric, how about this - if nothing else, that makes code in there simpler
>> and less dependent on details of VFS guts:
>
> Argh. No, it's not enough. Why are you using ->d_iput()? You are not
> doing anything unusual with inode; the natural place for that is in
> ->d_release() and then it will get simpler rules wrt setting ->d_fsdata.
No good reason. We do tie inode numbers to the syfs_dirent but the
inode was changed quite a while ago to hold it's own reference
sysfs_dirent. So using d_iput looks like sysfs historical baggage.
> As it is, you need to do that exactly after the point where you know
> that it dentry won't be dropped without going through d_add().
>
> OK, I've split that in two commits and put into vfs.git#sysfs; take a look
> and comment, please. Should get to git.kernel.in a few...
The patches on your sysfs branch look reasonable.
I am still learly of d_materialise_unique as it allows to create alias's
on non-directories. It isn't a functional problem as d_revalidate will
catch the issue and make it look like we have a unlink/link pair instead
of a proper rename. However since it is possible I would like to aim
for the higher quality of implemntation and use show renames as renames.
What would be ideal for sysfs is the d_add_singleton function below. It
does what is needed without the weird d_materialise strangeness that is
in d_materialise_unique. But if a all singing all dancing all
confusing function is preferable I would not mind.
What I would really like is an interface so that a distrubuted/remote
filesystem can provide an inotify like stream of and we can really
implement inotify in a distributed filesystem. But since I am too lazy
to do that I am reluctant to give up what progress I have actually made
in that direction.
Eric
diff --git a/fs/dcache.c b/fs/dcache.c
index 85c9e2b..2aab524 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2537,6 +2537,74 @@ out_nolock:
}
EXPORT_SYMBOL_GPL(d_materialise_unique);
+/**
+ * d_add_singleton - add an inode with only a single dentry
+ * @entry: dentry to instantiate
+ * @inode: inode to attach to this dentry
+ *
+ * Fill in inode information in the entry. On success, it returns NULL.
+ * If an alias of "entry" already exists, then we assume that a rename
+ * has occurred and not been reported so the alias is renamed and we
+ * return the aliased dentry and drop one reference to the inode.
+ *
+ * Note that in order to avoid conflicts with rename() etc, the caller
+ * had better be holding the parent directory semaphore.
+ *
+ * This also assumes that the inode count has been incremented
+ * (or otherwise set) by the caller to indicate that it is now
+ * in use by the dcache.
+ */
+struct dentry *d_add_singleton(struct dentry *entry, struct inode *inode)
+{
+ struct dentry *alias, *actual = entry;
+
+ if (!inode) {
+ __d_instantiate(entry, NULL);
+ d_rehash(entry);
+ goto out_nolock;
+ }
+
+ spin_lock(&inode->i_lock);
+
+ /* Does an aliased dentry already exist? */
+ alias = __d_find_alias(inode);
+ if (alias) {
+ write_seqlock(&rename_lock);
+
+ if (d_ancestor(alias, entry)) {
+ /* Check for loops */
+ actual = ERR_PTR(-ELOOP);
+ spin_unlock(&inode->i_lock);
+ } else {
+ /* Avoid aliases. This drops inode->i_lock */
+ actual = __d_unalias(inode, entry, alias);
+ }
+ write_sequnlock(&rename_lock);
+ if (IS_ERR(actual)) {
+ if (PTR_ERR(actual) == -ELOOP)
+ pr_warn_ratelimited(
+ "VFS: Lookup of '%s' in %s %s"
+ " would have caused loop\n",
+ entry->d_name.name,
+ inode->i_sb->s_type->name,
+ inode->i_sb->s_id);
+ dput(alias);
+ }
+ goto out_nolock;
+ }
+ __d_instantiate(entry, inode);
+ spin_unlock(&inode->i_lock);
+ d_rehash(entry);
+out_nolock:
+ if (actual == entry ) {
+ security_d_instantiate(entry, inode);
+ return NULL;
+ }
+ iput(inode);
+ return actual;
+}
+
+
static int prepend(char **buffer, int *buflen, const char *str, int namelen)
{
*buflen -= namelen;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 094789f..9613d4c 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -219,6 +219,8 @@ static inline int dname_external(struct dentry *dentry)
extern void d_instantiate(struct dentry *, struct inode *);
extern struct dentry * d_instantiate_unique(struct dentry *, struct inode *);
extern struct dentry * d_materialise_unique(struct dentry *, struct inode *);
+extern struct dentry * d_materialise_unalias(struct dentry *, struct inode *);
+extern struct dentry *d_add_singleton(struct dentry *, struct inode *);
extern void __d_drop(struct dentry *dentry);
extern void d_drop(struct dentry *dentry);
extern void d_delete(struct dentry *);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists