[stable] [PATCH] - fix oops in sysfs_readdir

[stable] [PATCH] - fix oops in sysfs_readdir

[Eric Sandeen]

This is a non-ida backport of Tejun's patch in -mm at:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
but uses a brain-dead-simple inode nr allocator rather than ida, which would
bring along a lot of newer, more complex code.

No, this doesn't guarantee uniqueness of sysfs inode numbers, but then
the code in -stable today doesn't either - and with this change, at least
it shouldn't oops.

Comments?

Thanks,

-Eric

Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Index: linux-2.6.16.51/fs/sysfs/dir.c
===================================================================
--- linux-2.6.16.51.orig/fs/sysfs/dir.c
+++ linux-2.6.16.51/fs/sysfs/dir.c
@@ -29,6 +29,14 @@ static struct dentry_operations sysfs_de
 	.d_iput		= sysfs_d_iput,
 };
 
+static unsigned int sysfs_inode_counter;
+ino_t sysfs_get_inum(void)
+{
+	if (unlikely(sysfs_inode_counter < 3))
+		sysfs_inode_counter = 3;
+	return sysfs_inode_counter++;
+}
+
 /*
  * Allocates a new sysfs_dirent and links it to the parent sysfs_dirent
  */
@@ -385,7 +393,7 @@ static int sysfs_readdir(struct file * f
 
 	switch (i) {
 		case 0:
-			ino = dentry->d_inode->i_ino;
+			ino = parent_sd->s_ino;
 			if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
 				break;
 			filp->f_pos++;
@@ -415,10 +423,7 @@ static int sysfs_readdir(struct file * f
 
 				name = sysfs_get_name(next);
 				len = strlen(name);
-				if (next->s_dentry)
-					ino = next->s_dentry->d_inode->i_ino;
-				else
-					ino = iunique(sysfs_sb, 2);
+				ino = next->s_ino;
 
 				if (filldir(dirent, name, len, filp->f_pos, ino,
 						 dt_type(next)) < 0)
Index: linux-2.6.16.51/fs/sysfs/inode.c
===================================================================
--- linux-2.6.16.51.orig/fs/sysfs/inode.c
+++ linux-2.6.16.51/fs/sysfs/inode.c
@@ -119,6 +119,7 @@ struct inode * sysfs_new_inode(mode_t mo
 		inode->i_mapping->a_ops = &sysfs_aops;
 		inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info;
 		inode->i_op = &sysfs_inode_operations;
+		inode->i_ino = sd->s_ino;
 
 		if (sd->s_iattr) {
 			/* sysfs_dirent has non-default attributes
Index: linux-2.6.16.51/fs/sysfs/mount.c
===================================================================
--- linux-2.6.16.51.orig/fs/sysfs/mount.c
+++ linux-2.6.16.51/fs/sysfs/mount.c
@@ -29,6 +29,7 @@ static struct sysfs_dirent sysfs_root = 
 	.s_element	= NULL,
 	.s_type		= SYSFS_ROOT,
 	.s_iattr	= NULL,
+	.s_ino		= 1,
 };
 
 static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
Index: linux-2.6.16.51/include/linux/sysfs.h
===================================================================
--- linux-2.6.16.51.orig/include/linux/sysfs.h
+++ linux-2.6.16.51/include/linux/sysfs.h
@@ -72,6 +72,7 @@ struct sysfs_dirent {
 	void 			* s_element;
 	int			s_type;
 	umode_t			s_mode;
+	ino_t			s_ino;
 	struct dentry		* s_dentry;
 	struct iattr		* s_iattr;
 };

Re: [PATCH] - fix oops in sysfs_readdir

[Chris Wright]

[Please Cc: Adrian Bunk (added) for 2.6.16-stable material]

* Eric Sandeen (sandeen@redhat.com) wrote:
> This is a non-ida backport of Tejun's patch in -mm at:
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
> for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
> store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
> but uses a brain-dead-simple inode nr allocator rather than ida, which would
> bring along a lot of newer, more complex code.
> 
> No, this doesn't guarantee uniqueness of sysfs inode numbers, but then
> the code in -stable today doesn't either - and with this change, at least
> it shouldn't oops.
> 
> Comments?
> 
> Thanks,
> 
> -Eric
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> Index: linux-2.6.16.51/fs/sysfs/dir.c
> ===================================================================
> --- linux-2.6.16.51.orig/fs/sysfs/dir.c
> +++ linux-2.6.16.51/fs/sysfs/dir.c
> @@ -29,6 +29,14 @@ static struct dentry_operations sysfs_de
>  	.d_iput		= sysfs_d_iput,
>  };
>  
> +static unsigned int sysfs_inode_counter;
> +ino_t sysfs_get_inum(void)
> +{
> +	if (unlikely(sysfs_inode_counter < 3))
> +		sysfs_inode_counter = 3;
> +	return sysfs_inode_counter++;
> +}
> +
>  /*
>   * Allocates a new sysfs_dirent and links it to the parent sysfs_dirent
>   */
> @@ -385,7 +393,7 @@ static int sysfs_readdir(struct file * f
>  
>  	switch (i) {
>  		case 0:
> -			ino = dentry->d_inode->i_ino;
> +			ino = parent_sd->s_ino;
>  			if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
>  				break;
>  			filp->f_pos++;
> @@ -415,10 +423,7 @@ static int sysfs_readdir(struct file * f
>  
>  				name = sysfs_get_name(next);
>  				len = strlen(name);
> -				if (next->s_dentry)
> -					ino = next->s_dentry->d_inode->i_ino;
> -				else
> -					ino = iunique(sysfs_sb, 2);
> +				ino = next->s_ino;
>  
>  				if (filldir(dirent, name, len, filp->f_pos, ino,
>  						 dt_type(next)) < 0)
> Index: linux-2.6.16.51/fs/sysfs/inode.c
> ===================================================================
> --- linux-2.6.16.51.orig/fs/sysfs/inode.c
> +++ linux-2.6.16.51/fs/sysfs/inode.c
> @@ -119,6 +119,7 @@ struct inode * sysfs_new_inode(mode_t mo
>  		inode->i_mapping->a_ops = &sysfs_aops;
>  		inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info;
>  		inode->i_op = &sysfs_inode_operations;
> +		inode->i_ino = sd->s_ino;
>  
>  		if (sd->s_iattr) {
>  			/* sysfs_dirent has non-default attributes
> Index: linux-2.6.16.51/fs/sysfs/mount.c
> ===================================================================
> --- linux-2.6.16.51.orig/fs/sysfs/mount.c
> +++ linux-2.6.16.51/fs/sysfs/mount.c
> @@ -29,6 +29,7 @@ static struct sysfs_dirent sysfs_root = 
>  	.s_element	= NULL,
>  	.s_type		= SYSFS_ROOT,
>  	.s_iattr	= NULL,
> +	.s_ino		= 1,
>  };
>  
>  static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
> Index: linux-2.6.16.51/include/linux/sysfs.h
> ===================================================================
> --- linux-2.6.16.51.orig/include/linux/sysfs.h
> +++ linux-2.6.16.51/include/linux/sysfs.h
> @@ -72,6 +72,7 @@ struct sysfs_dirent {
>  	void 			* s_element;
>  	int			s_type;
>  	umode_t			s_mode;
> +	ino_t			s_ino;
>  	struct dentry		* s_dentry;
>  	struct iattr		* s_iattr;
>  };

Re: [stable] [PATCH] - fix oops in sysfs_readdir

[Eric Sandeen]

Eric Sandeen wrote:

> This is a non-ida backport of Tejun's patch in -mm at:
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
> for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
> store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
> but uses a brain-dead-simple inode nr allocator rather than ida, which would
> bring along a lot of newer, more complex code.
>
> No, this doesn't guarantee uniqueness of sysfs inode numbers, but then
> the code in -stable today doesn't either - and with this change, at least
> it shouldn't oops.
>
> Comments?
>
> Thanks,
>
> -Eric
Here's one that works w/2.6.21... the global should probably be an atomic 
or lock-protected, I suppose, but again, if we don't guarantee uniqueness
anyway...?

Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Index: linux-2.6.21/fs/sysfs/dir.c

===================================================================
--- linux-2.6.21.orig/fs/sysfs/dir.c
+++ linux-2.6.21/fs/sysfs/dir.c
@@ -32,6 +32,14 @@ static struct dentry_operations sysfs_de
 	.d_iput		= sysfs_d_iput,
 };
 
+static unsigned int sysfs_inode_counter;
+ino_t sysfs_get_inum(void)
+{
+	if (unlikely(sysfs_inode_counter < 3))
+		sysfs_inode_counter = 3;
+	return sysfs_inode_counter++;
+}
+
 /*
  * Allocates a new sysfs_dirent and links it to the parent sysfs_dirent
  */
@@ -511,7 +519,7 @@ static int sysfs_readdir(struct file * f
 
 	switch (i) {
 		case 0:
-			ino = dentry->d_inode->i_ino;
+			ino = parent_sd->s_ino;
 			if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
 				break;
 			filp->f_pos++;
@@ -540,10 +548,7 @@ static int sysfs_readdir(struct file * f
 
 				name = sysfs_get_name(next);
 				len = strlen(name);
-				if (next->s_dentry)
-					ino = next->s_dentry->d_inode->i_ino;
-				else
-					ino = iunique(sysfs_sb, 2);
+				ino = next->s_ino;
 
 				if (filldir(dirent, name, len, filp->f_pos, ino,
 						 dt_type(next)) < 0)
Index: linux-2.6.21/fs/sysfs/inode.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/inode.c
+++ linux-2.6.21/fs/sysfs/inode.c
@@ -140,6 +140,7 @@ struct inode * sysfs_new_inode(mode_t mo
 		inode->i_mapping->a_ops = &sysfs_aops;
 		inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info;
 		inode->i_op = &sysfs_inode_operations;
+		inode->i_ino = sd->s_ino;
 		lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
 
 		if (sd->s_iattr) {
Index: linux-2.6.21/fs/sysfs/mount.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/mount.c
+++ linux-2.6.21/fs/sysfs/mount.c
@@ -33,6 +33,7 @@ static struct sysfs_dirent sysfs_root = 
 	.s_element	= NULL,
 	.s_type		= SYSFS_ROOT,
 	.s_iattr	= NULL,
+	.s_ino		= 1,
 };
 
 static void sysfs_clear_inode(struct inode *inode)
Index: linux-2.6.21/fs/sysfs/sysfs.h
===================================================================
--- linux-2.6.21.orig/fs/sysfs/sysfs.h
+++ linux-2.6.21/fs/sysfs/sysfs.h
@@ -5,6 +5,7 @@ struct sysfs_dirent {
 	void 			* s_element;
 	int			s_type;
 	umode_t			s_mode;
+	ino_t			s_ino;
 	struct dentry		* s_dentry;
 	struct iattr		* s_iattr;
 	atomic_t		s_event;

Re: [stable] [PATCH] - fix oops in sysfs_readdir

[Andrew Morton]

On Mon, 21 May 2007 13:11:21 -0500
Eric Sandeen <sandeen@redhat.com> wrote:

> This is a non-ida backport of Tejun's patch in -mm at:
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
> for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
> store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
> but uses a brain-dead-simple inode nr allocator rather than ida, which would
> bring along a lot of newer, more complex code.
> 
> No, this doesn't guarantee uniqueness of sysfs inode numbers, but then
> the code in -stable today doesn't either - and with this change, at least
> it shouldn't oops.

So I'm sitting here whether to commend this patch to google kernel maintainers
for 2.6.18 backport, but I realise I don't know what it does.  And I don't know
if it fixes the reclaim-time oopses they were intermittently seeing, or if it
fixes something else and if so what that is.

Sigh.  Better changelogs, please.

Re: [stable] [PATCH] - fix oops in sysfs_readdir

[Eric Sandeen]

Andrew Morton wrote:
> On Mon, 21 May 2007 13:11:21 -0500
> Eric Sandeen <sandeen@redhat.com> wrote:
> 
>> This is a non-ida backport of Tejun's patch in -mm at:
>> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
>> for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
>> store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
>> but uses a brain-dead-simple inode nr allocator rather than ida, which would
>> bring along a lot of newer, more complex code.
>>
>> No, this doesn't guarantee uniqueness of sysfs inode numbers, but then
>> the code in -stable today doesn't either - and with this change, at least
>> it shouldn't oops.
> 
> So I'm sitting here whether to commend this patch to google kernel maintainers
> for 2.6.18 backport, but I realise I don't know what it does.  And I don't know
> if it fixes the reclaim-time oopses they were intermittently seeing, or if it
> fixes something else and if so what that is.
> 
> Sigh.  Better changelogs, please.
> 

Sorry Andrew.  I referenced Tejun's upstream patch in -mm which has a 
nice changelog etc, and this is a backport of that, and does the same 
thing in the same way and solves the same problem - but that doesn't 
help if you just want to toss this message into your patch stack.  Will 
fix up & resend.

-Eric

-Eric

Re: [stable] [PATCH] - fix oops in sysfs_readdir

[Andrew Morton]

On Mon, 21 May 2007 19:18:55 -0500 Eric Sandeen <sandeen@redhat.com> wrote:

> Andrew Morton wrote:
> > On Mon, 21 May 2007 13:11:21 -0500
> > Eric Sandeen <sandeen@redhat.com> wrote:
> > 
> >> This is a non-ida backport of Tejun's patch in -mm at:
> >> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
> >> for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
> >> store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
> >> but uses a brain-dead-simple inode nr allocator rather than ida, which would
> >> bring along a lot of newer, more complex code.
> >>
> >> No, this doesn't guarantee uniqueness of sysfs inode numbers, but then
> >> the code in -stable today doesn't either - and with this change, at least
> >> it shouldn't oops.
> > 
> > So I'm sitting here whether to commend this patch to google kernel maintainers
> > for 2.6.18 backport, but I realise I don't know what it does.  And I don't know
> > if it fixes the reclaim-time oopses they were intermittently seeing, or if it
> > fixes something else and if so what that is.
> > 
> > Sigh.  Better changelogs, please.
> > 
> 
> Sorry Andrew.  I referenced Tejun's upstream patch in -mm which has a 
> nice changelog etc, and this is a backport of that, and does the same 
> thing in the same way and solves the same problem - but that doesn't 
> help if you just want to toss this message into your patch stack.  Will 
> fix up & resend.
> 

Actually, someone (eg distros) looking at Tejun's changelog would still be
struggling to answer the question "do I need this".  The one thing it
claims to fix is "duplicate inode numbers".  But why is that a problem? 
What are the user-visible consequences of not merging the patch?  Unobvious.

Re: [stable] [PATCH] - fix oops in sysfs_readdir

[Tejun Heo]

Andrew Morton wrote:
> Actually, someone (eg distros) looking at Tejun's changelog would still be
> struggling to answer the question "do I need this".  The one thing it
> claims to fix is "duplicate inode numbers".  But why is that a problem? 
> What are the user-visible consequences of not merging the patch?  Unobvious.

The oops part is explained in #2.  sysfs_dirent->s_dentry can go away
anytime and the original code accesses it without any synchronization,
so it can end up dereferencing NULL or access already freed memory.
And, yeah, this is another place where reclaim-related oops occurs.

-- 
tejun

Re: [stable] [PATCH] - store sysfs inode nrs in s_ino to avoid readdir oopses

[Eric Sandeen]

(2nd try, better(?) changelog, quilt refreshed(!) patch)

--

Backport of
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch

For regular files in sysfs, sysfs_readdir wants to traverse
sysfs_dirent->s_dentry->d_inode->i_ino to get to the inode number.
But, the dentry can be reclaimed under memory pressure, and there
is no synchronization with readdir.  This patch follows Tejun's
scheme of allocating and storing an inode number in the new s_ino
member of a sysfs_dirent, when dirents are created, and retrieving 
it from there for readdir, so that the pointer chain doesn't have 
to be traversed.

Tejun's upstream patch uses a new-ish "ida" allocator which brings along
some extra complexity; this -stable patch has a brain-dead incrementing
counter which does not guarantee uniqueness, but because sysfs doesn't 
hash inodes as iunique expects, uniqueness wasn't guaranteed today anyway.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Index: linux-2.6.21/fs/sysfs/dir.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/dir.c
+++ linux-2.6.21/fs/sysfs/dir.c
@@ -30,6 +30,14 @@ static struct dentry_operations sysfs_de
 	.d_iput		= sysfs_d_iput,
 };
 
+static unsigned int sysfs_inode_counter;
+ino_t sysfs_get_inum(void)
+{
+	if (unlikely(sysfs_inode_counter < 3))
+		sysfs_inode_counter = 3;
+	return sysfs_inode_counter++;
+}
+
 /*
  * Allocates a new sysfs_dirent and links it to the parent sysfs_dirent
  */
@@ -41,6 +49,7 @@ static struct sysfs_dirent * __sysfs_new
 	if (!sd)
 		return NULL;
 
+	sd->s_ino = sysfs_get_inum();
 	atomic_set(&sd->s_count, 1);
 	atomic_set(&sd->s_event, 1);
 	INIT_LIST_HEAD(&sd->s_children);
@@ -509,7 +518,7 @@ static int sysfs_readdir(struct file * f
 
 	switch (i) {
 		case 0:
-			ino = dentry->d_inode->i_ino;
+			ino = parent_sd->s_ino;
 			if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
 				break;
 			filp->f_pos++;
@@ -538,10 +547,7 @@ static int sysfs_readdir(struct file * f
 
 				name = sysfs_get_name(next);
 				len = strlen(name);
-				if (next->s_dentry)
-					ino = next->s_dentry->d_inode->i_ino;
-				else
-					ino = iunique(sysfs_sb, 2);
+				ino = next->s_ino;
 
 				if (filldir(dirent, name, len, filp->f_pos, ino,
 						 dt_type(next)) < 0)
Index: linux-2.6.21/fs/sysfs/inode.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/inode.c
+++ linux-2.6.21/fs/sysfs/inode.c
@@ -140,6 +140,7 @@ struct inode * sysfs_new_inode(mode_t mo
 		inode->i_mapping->a_ops = &sysfs_aops;
 		inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info;
 		inode->i_op = &sysfs_inode_operations;
+		inode->i_ino = sd->s_ino;
 		lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
 
 		if (sd->s_iattr) {
Index: linux-2.6.21/fs/sysfs/mount.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/mount.c
+++ linux-2.6.21/fs/sysfs/mount.c
@@ -33,6 +33,7 @@ static struct sysfs_dirent sysfs_root = 
 	.s_element	= NULL,
 	.s_type		= SYSFS_ROOT,
 	.s_iattr	= NULL,
+	.s_ino		= 1,
 };
 
 static void sysfs_clear_inode(struct inode *inode)
Index: linux-2.6.21/fs/sysfs/sysfs.h
===================================================================
--- linux-2.6.21.orig/fs/sysfs/sysfs.h
+++ linux-2.6.21/fs/sysfs/sysfs.h
@@ -5,6 +5,7 @@ struct sysfs_dirent {
 	void 			* s_element;
 	int			s_type;
 	umode_t			s_mode;
+	ino_t			s_ino;
 	struct dentry		* s_dentry;
 	struct iattr		* s_iattr;
 	atomic_t		s_event;

Re: [stable] [PATCH] - fix oops in sysfs_readdir

[Adrian Bunk]

On Mon, May 21, 2007 at 01:11:21PM -0500, Eric Sandeen wrote:
> This is a non-ida backport of Tejun's patch in -mm at:
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
> for the 2.6.16 -stable tree - it follows the same scheme of using s_ino to safely
> store & retrieve the inode number of sysfs entries for use in sysfs_readdir,
> but uses a brain-dead-simple inode nr allocator rather than ida, which would
> bring along a lot of newer, more complex code.
> 
> No, this doesn't guarantee uniqueness of sysfs inode numbers, but then
> the code in -stable today doesn't either - and with this change, at least
> it shouldn't oops.
> 
> Comments?

First of all, thanks for any contributions to 2.6.16.

My biggest problem with this patch is:
It's not yet fixed in Linus' tree - and if it isn't important enough for 
being fixed in 2.6.22, it can't be important enough for 2.6.16.

And no matter whether this might include adding ida to 2.6.16, I'd 
prefer to apply something as near as possible to whatever gets
into 2.6.22.

> Thanks,
> 
> -Eric
>...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

patch sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch queued to -stable tree

[gregkh]

This is a note to let you know that we have just queued up the patch titled

     Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses

to the 2.6.21-stable tree.  Its filename is

     sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch

A git repo of this tree can be found at 
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary


>From stable-bounces@linux.kernel.org Mon May 21 19:34:01 2007
From: Eric Sandeen <sandeen@sandeen.net>
Date: Mon, 21 May 2007 21:32:40 -0500
Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses
To: Eric Sandeen <sandeen@redhat.com>
Cc: stable@kernel.org, Andrew Morton <akpm@linux-foundation.org>, Tejun Heo <htejun@gmail.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Maneesh Soni <maneesh@in.ibm.com>
Message-ID: <46525648.8050807@sandeen.net>


Backport of
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch

For regular files in sysfs, sysfs_readdir wants to traverse
sysfs_dirent->s_dentry->d_inode->i_ino to get to the inode number.
But, the dentry can be reclaimed under memory pressure, and there
is no synchronization with readdir.  This patch follows Tejun's
scheme of allocating and storing an inode number in the new s_ino
member of a sysfs_dirent, when dirents are created, and retrieving 
it from there for readdir, so that the pointer chain doesn't have 
to be traversed.

Tejun's upstream patch uses a new-ish "ida" allocator which brings along
some extra complexity; this -stable patch has a brain-dead incrementing
counter which does not guarantee uniqueness, but because sysfs doesn't 
hash inodes as iunique expects, uniqueness wasn't guaranteed today anyway.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Cc: Tejun Heo <htejun@gmail.com>
Cc: Maneesh Soni <maneesh@in.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

--- linux-2.6.21.orig/fs/sysfs/dir.c
+++ linux-2.6.21/fs/sysfs/dir.c
@@ -30,6 +30,14 @@ static struct dentry_operations sysfs_de
 	.d_iput		= sysfs_d_iput,
 };
 
+static unsigned int sysfs_inode_counter;
+ino_t sysfs_get_inum(void)
+{
+	if (unlikely(sysfs_inode_counter < 3))
+		sysfs_inode_counter = 3;
+	return sysfs_inode_counter++;
+}
+
 /*
  * Allocates a new sysfs_dirent and links it to the parent sysfs_dirent
  */
@@ -41,6 +49,7 @@ static struct sysfs_dirent * __sysfs_new
 	if (!sd)
 		return NULL;
 
+	sd->s_ino = sysfs_get_inum();
 	atomic_set(&sd->s_count, 1);
 	atomic_set(&sd->s_event, 1);
 	INIT_LIST_HEAD(&sd->s_children);
@@ -509,7 +518,7 @@ static int sysfs_readdir(struct file * f
 
 	switch (i) {
 		case 0:
-			ino = dentry->d_inode->i_ino;
+			ino = parent_sd->s_ino;
 			if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
 				break;
 			filp->f_pos++;
@@ -538,10 +547,7 @@ static int sysfs_readdir(struct file * f
 
 				name = sysfs_get_name(next);
 				len = strlen(name);
-				if (next->s_dentry)
-					ino = next->s_dentry->d_inode->i_ino;
-				else
-					ino = iunique(sysfs_sb, 2);
+				ino = next->s_ino;
 
 				if (filldir(dirent, name, len, filp->f_pos, ino,
 						 dt_type(next)) < 0)
Index: linux-2.6.21/fs/sysfs/inode.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/inode.c
+++ linux-2.6.21/fs/sysfs/inode.c
@@ -140,6 +140,7 @@ struct inode * sysfs_new_inode(mode_t mo
 		inode->i_mapping->a_ops = &sysfs_aops;
 		inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info;
 		inode->i_op = &sysfs_inode_operations;
+		inode->i_ino = sd->s_ino;
 		lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key);
 
 		if (sd->s_iattr) {
Index: linux-2.6.21/fs/sysfs/mount.c
===================================================================
--- linux-2.6.21.orig/fs/sysfs/mount.c
+++ linux-2.6.21/fs/sysfs/mount.c
@@ -33,6 +33,7 @@ static struct sysfs_dirent sysfs_root = 
 	.s_element	= NULL,
 	.s_type		= SYSFS_ROOT,
 	.s_iattr	= NULL,
+	.s_ino		= 1,
 };
 
 static void sysfs_clear_inode(struct inode *inode)
Index: linux-2.6.21/fs/sysfs/sysfs.h
===================================================================
--- linux-2.6.21.orig/fs/sysfs/sysfs.h
+++ linux-2.6.21/fs/sysfs/sysfs.h
@@ -5,6 +5,7 @@ struct sysfs_dirent {
 	void 			* s_element;
 	int			s_type;
 	umode_t			s_mode;
+	ino_t			s_ino;
 	struct dentry		* s_dentry;
 	struct iattr		* s_iattr;
 	atomic_t		s_event;

_______________________________________________
stable mailing list
stable@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/stable


Patches currently in stable-queue which might be from sandeen@sandeen.net are

queue-2.6.21/sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch

Re: [stable] patch sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch queued to -stable tree

[Chris Wright]

* gregkh@suse.de (gregkh@suse.de) wrote:
> From: Eric Sandeen <sandeen@sandeen.net>
> Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses
> 
> Backport of
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
> 

I didn't put this into -stable queue because the ida version is not
upstream yet, so I don't think it's appropriate to backport it at this
point in time.

thanks,
-chris

Re: [stable] patch sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch queued to -stable tree

[Eric Sandeen]

Chris Wright wrote:
> * gregkh@suse.de (gregkh@suse.de) wrote:
>> From: Eric Sandeen <sandeen@sandeen.net>
>> Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses
>>
>> Backport of
>> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
>>
> 
> I didn't put this into -stable queue because the ida version is not
> upstream yet, so I don't think it's appropriate to backport it at this
> point in time.

Well, my backport of Tejun's patch explicitly doesn't use ida for just
that reason...

It uses a simple counter instead (which may give dup inode numbers, but
I think we have that today, and at least this shouldn't oops...)

*shrug*

-Eric

> thanks,
> -chris
> 

Re: [stable] patch sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch queued to -stable tree

[Chris Wright]

* Eric Sandeen (sandeen@sandeen.net) wrote:
> Well, my backport of Tejun's patch explicitly doesn't use ida for just
> that reason...
> 
> It uses a simple counter instead (which may give dup inode numbers, but
> I think we have that today, and at least this shouldn't oops...)

Right, my point is the source of the backport is in -mm for maybe as
long as 6 weeks but not in Linus' tree.  I s'pose it's chicken and egg
for the ida code.  I'd prefer to see your patch upstream, and Tejun's
code becomes the ida code plus conversion of the simple counter to ida
at some later point in time.

Re: [stable] patch sysfs-store-sysfs-inode-nrs-in-s_ino-to-avoid-readdir-oopses.patch queued to -stable tree

[Greg KH]

On Wed, Jun 06, 2007 at 04:36:22PM -0500, Eric Sandeen wrote:
> Chris Wright wrote:
> > * gregkh@suse.de (gregkh@suse.de) wrote:
> >> From: Eric Sandeen <sandeen@sandeen.net>
> >> Subject: sysfs: store sysfs inode nrs in s_ino to avoid readdir oopses
> >>
> >> Backport of
> >> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/broken-out/gregkh-driver-sysfs-allocate-inode-number-using-ida.patch
> >>
> > 
> > I didn't put this into -stable queue because the ida version is not
> > upstream yet, so I don't think it's appropriate to backport it at this
> > point in time.
> 
> Well, my backport of Tejun's patch explicitly doesn't use ida for just
> that reason...
> 
> It uses a simple counter instead (which may give dup inode numbers, but
> I think we have that today, and at least this shouldn't oops...)

Ok, I'll pull it out as the number of people really seeing this in the
wild is so small that it's not worth it.

thanks,

greg k-h