| 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
| ||
|
Date: Tue, 2 Feb 2010 17:01:57 +0000
From: Al Viro <viro@...IV.linux.org.uk>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH][RFC] %pd - for printing dentry name
On Mon, Feb 01, 2010 at 09:55:42PM -0800, Eric W. Biederman wrote:
> We already have rename_lock, which is only take for write in d_move_locked.
>
> I wonder if there is an instruction sequence that could guarantee that the
> string copy is done atomically from the perspective of another cpu,
> d_iname fits nicely on a single cache line so it should be possible.
>
> That is a stronger guarantee than we need. All we really need is the
> guarantee that a reader will see the string null terminator. dentries already
> have rcu safe lifetimes.
>
> Hmm.
>
> We should be able to do:
> struct qstr *name;
> int len;
> char buf[MAX_LEN + 1];
> long seq;
>
> do {
> seq = read_seqbegin(&rename_lock);
> rcu_read_lock();
> name = rcu_dereference(dentry->d_name.name);
> len = dentry->d_name.len;
> if (read_seqretry(&rename_lock, seq)
> continue;
> if (len > MAX_LEN)
> len = MAX_LEN;
> memcpy(buf, name, len);
> buf[len] = '\0';
> rcu_read_unlock();
> } while (read_seqretry(&rename_lock, seq));
Actually, WTF do we bother? seqlock writer grabs embedded spinlock.
So can we, since *that* is far more narrow than ->d_lock and it's
static, so it's not like somebody outside of dcache.c can decide
to grab.
We could just inline string() into dname_string() and take the entire
thing to fs/dcache.c. And protect it either with direct locking
of rename_lock.lock or another seq_writelock; contention is not
an issue, since if printk guts are your hotpath you are already
FUBAR.
Something like that (completely untested):
diff --git a/fs/dcache.c b/fs/dcache.c
index 953173a..a4d30bc 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1703,6 +1703,48 @@ void d_move(struct dentry * dentry, struct dentry * target)
spin_unlock(&dcache_lock);
}
+/*
+ * for vsprintf use only.
+ */
+char *dname_string(char *buf, char *end, struct dentry *d,
+ int precision, int width, int left_align)
+{
+ int len, i;
+ const unsigned char *s;
+
+ write_seqlock(&rename_lock);
+ if (precision > d->d_name.len)
+ precision = d->d_name.len;
+
+ s = d->d_name.name;
+ if ((unsigned long)s < PAGE_SIZE)
+ s = "(null)";
+
+ len = strnlen(s, precision);
+
+ if (!left_align) {
+ while (len < width--) {
+ if (buf < end)
+ *buf = ' ';
+ ++buf;
+ }
+ }
+ for (i = 0; i < len; ++i) {
+ if (buf < end)
+ *buf = *s;
+ ++buf; ++s;
+ }
+
+ while (len < width--) {
+ if (buf < end)
+ *buf = ' ';
+ ++buf;
+ }
+
+ write_sequnlock(&rename_lock);
+ return buf;
+}
+
/**
* d_ancestor - search for an ancestor
* @p1: ancestor dentry
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 30b93b2..2dc286a 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -379,5 +379,6 @@ extern struct vfsmount *lookup_mnt(struct path *);
extern struct dentry *lookup_create(struct nameidata *nd, int is_dir);
extern int sysctl_vfs_cache_pressure;
+extern char *dname_string(char *, char *, struct dentry *, int, int, int);
#endif /* __LINUX_DCACHE_H */
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3b8aeec..ab0e40a 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -915,6 +915,8 @@ static char *uuid_string(char *buf, char *end, const u8 *addr,
* [0][1][2][3]-[4][5]-[6][7]-[8][9]-[10][11][12][13][14][15]
* little endian output byte order is:
* [3][2][1][0]-[5][4]-[7][6]-[8][9]-[10][11][12][13][14][15]
+ * - 'd' For dentry name. NOTE: don't use under dentry->d_lock, there
+ * you can safely use ->d_name.name instead.
*
* Note: The difference between 'S' and 'F' is that on ia64 and ppc64
* function pointers are really function descriptors, which contain a
@@ -958,6 +960,9 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
break;
case 'U':
return uuid_string(buf, end, ptr, spec, fmt);
+ case 'd':
+ return dname_string(buf, end, ptr, spec.precision,
+ spec.field_width, spec.flags & LEFT);
}
spec.flags |= SMALL;
if (spec.field_width == -1) {
@@ -1191,6 +1196,7 @@ qualifier:
* http://tools.ietf.org/html/draft-ietf-6man-text-addr-representation-00
* %pU[bBlL] print a UUID/GUID in big or little endian using lower or upper
* case.
+ * %pd print dentry name
* %n is ignored
*
* The return value is the number of characters which would
--
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