2007-01-30 15:38:54

by Jeff Layton

[permalink] [raw]
Subject: [PATCH 2/2] make pipefs do lazy i_ino assignment and hashing

This patch updates pipefs to do defer assigning an i_ino value to its inodes
until someone actually tries to stat it. This allows us to have unique i_ino
values for the inodes here, without the performance impact for anyone who
doesn't actually care about it.

Since we don't have an i_ino value at pipe creation time, we need something
else to stuff into the dentry name. Here, I'm using the pointer address of
the inode xor'ed with a random value. There are certainly better hashing
schemes, so if someone wants to propose a better way to do this, then I'm
open to looking at it (maybe halfmd4?).

This patch should apply cleanly to the current -mm.

Signed-off-by: Jeff Layton <[email protected]>

diff --git a/fs/pipe.c b/fs/pipe.c
index 9b3cb34..76dc0e1 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -16,6 +16,7 @@
#include <linux/uio.h>
#include <linux/highmem.h>
#include <linux/pagemap.h>
+#include <linux/random.h>

#include <asm/uaccess.h>
#include <asm/ioctls.h>
@@ -35,6 +36,8 @@
* -- Manfred Spraul <[email protected]> 2002-05-09
*/

+static unsigned long pipefs_dname_obfuscator;
+
/* Drop the inode semaphore and wait for a pipe event, atomically */
void pipe_wait(struct pipe_inode_info *pipe)
{
@@ -844,6 +847,10 @@ static struct dentry_operations pipefs_dentry_operations = {
.d_delete = pipefs_delete_dentry,
};

+static struct inode_operations pipefs_inode_operations = {
+ .getattr = lazy_getattr,
+};
+
static struct inode * get_pipe_inode(void)
{
struct inode *inode = new_inode(pipe_mnt->mnt_sb);
@@ -859,6 +866,7 @@ static struct inode * get_pipe_inode(void)

pipe->readers = pipe->writers = 1;
inode->i_fop = &rdwr_pipe_fops;
+ inode->i_op = &pipefs_inode_operations;

/*
* Mark the inode dirty from the very beginning,
@@ -871,8 +879,7 @@ static struct inode * get_pipe_inode(void)
inode->i_uid = current->fsuid;
inode->i_gid = current->fsgid;
inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
- inode->i_ino = iunique(pipe_mnt->mnt_sb, 1);
- insert_inode_hash(inode);
+ inode->i_ino = 0;

return inode;

@@ -900,7 +907,8 @@ struct file *create_write_pipe(void)
if (!inode)
goto err_file;

- this.len = sprintf(name, "[%lu]", inode->i_ino);
+ this.len = sprintf(name, "[%lu]",
+ (unsigned long) inode ^ pipefs_dname_obfuscator);
this.name = name;
this.hash = 0;
err = -ENOMEM;
@@ -1033,6 +1041,8 @@ static int __init init_pipe_fs(void)
{
int err = register_filesystem(&pipe_fs_type);

+ get_random_bytes(&pipefs_dname_obfuscator, sizeof(unsigned long));
+
if (!err) {
pipe_mnt = kern_mount(&pipe_fs_type);
if (IS_ERR(pipe_mnt)) {


2007-01-30 16:32:04

by Bodo Eggert

[permalink] [raw]
Subject: Re: [PATCH 2/2] make pipefs do lazy i_ino assignment and hashing

Jeff Layton <[email protected]> wrote:

> This patch updates pipefs to do defer assigning an i_ino value to its inodes
> until someone actually tries to stat it. This allows us to have unique i_ino
> values for the inodes here, without the performance impact for anyone who
> doesn't actually care about it.
>
> Since we don't have an i_ino value at pipe creation time, we need something
> else to stuff into the dentry name. Here, I'm using the pointer address of
> the inode xor'ed with a random value. There are certainly better hashing
> schemes, so if someone wants to propose a better way to do this, then I'm
> open to looking at it (maybe halfmd4?).

Why XOR? To pretend a non-existent level of security?
Either you can't use the address, or you can read the obfusicator value, too.

OTOH, if sizeof(void*) <= sieof(ino_t), using the address will result in a
unique inode number without need for expensive hashing algorithms.
--
Why did the hacker cross the road? To get to the other side.
Why did the cracker cross the road? To get what was on the other side.
The difference is small, but important.
-- Gandalf Parker