Re: [PATCH] Fix stack corruption in ext2fs server
On Wed, Jul 04, 2012 at 07:58:13AM -0300, Samuel Thibault wrote: > Richard Braun, le Wed 04 Jul 2012 08:22:13 +0200, a écrit : > > It's also quite a lot of heap too, and every 5 seconds. We should > > consider increasing the interval between global syncs to reduce the > > impact of this potentially heavy operation. > > It would increase the amount of work you loose on crash. Did you measure > the CPU use that you get with a typical filesystem, by e.g. touching a > file every 4 seconds to trigger the sync all the time? No. > I guess the real fix would be to be better at knowing which inodes need > to be recorded, and filter on that. Yes, hence the comment in my patch, but I leave that as an exercice for another motivated hacker. -- Richard Braun
Re: [PATCH] Fix stack corruption in ext2fs server
Richard Braun, le Wed 04 Jul 2012 08:22:13 +0200, a écrit : > On Tue, Jul 03, 2012 at 09:13:36PM -0300, Samuel Thibault wrote: > > Richard Braun, le Tue 03 Jul 2012 23:13:26 +0200, a écrit : > > > * ext2fs/inode.c (diskfs_node_iterate): allocate the temporary node > > > table from the heap instead of the stack. > > > > That can be quite a lot for the stack indeed, thanks! > > It's also quite a lot of heap too, and every 5 seconds. We should > consider increasing the interval between global syncs to reduce the > impact of this potentially heavy operation. It would increase the amount of work you loose on crash. Did you measure the CPU use that you get with a typical filesystem, by e.g. touching a file every 4 seconds to trigger the sync all the time? I guess the real fix would be to be better at knowing which inodes need to be recorded, and filter on that. Samuel
Re: [PATCH] Fix stack corruption in ext2fs server
On Tue, Jul 03, 2012 at 09:13:36PM -0300, Samuel Thibault wrote: > Richard Braun, le Tue 03 Jul 2012 23:13:26 +0200, a écrit : > > * ext2fs/inode.c (diskfs_node_iterate): allocate the temporary node > > table from the heap instead of the stack. > > That can be quite a lot for the stack indeed, thanks! It's also quite a lot of heap too, and every 5 seconds. We should consider increasing the interval between global syncs to reduce the impact of this potentially heavy operation. -- Richard Braun
Re: [PATCH] Fix stack corruption in ext2fs server
Richard Braun, le Tue 03 Jul 2012 23:13:26 +0200, a écrit : > * ext2fs/inode.c (diskfs_node_iterate): allocate the temporary node > table from the heap instead of the stack. That can be quite a lot for the stack indeed, thanks! Samuel
[PATCH] Fix stack corruption in ext2fs server
* ext2fs/inode.c (diskfs_node_iterate): allocate the temporary node
table from the heap instead of the stack.
Signed-off-by: Richard Braun
---
ext2fs/inode.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/ext2fs/inode.c b/ext2fs/inode.c
index f25cc1f..2da8a95 100644
--- a/ext2fs/inode.c
+++ b/ext2fs/inode.c
@@ -552,7 +552,16 @@ diskfs_node_iterate (error_t (*fun)(struct node *))
for (node = nodehash[n]; node; node = node->dn->hnext)
num_nodes++;
- node_list = alloca (num_nodes * sizeof (struct node *));
+ /* TODO This method doesn't scale beyond a few dozen nodes and should be
+ replaced. */
+ node_list = malloc (num_nodes * sizeof (struct node *));
+ if (node_list == NULL)
+{
+ spin_unlock (&diskfs_node_refcnt_lock);
+ ext2_debug ("unable to allocate temporary node table");
+ return ENOMEM;
+}
+
p = node_list;
for (n = 0; n < INOHSZ; n++)
for (node = nodehash[n]; node; node = node->dn->hnext)
@@ -576,6 +585,7 @@ diskfs_node_iterate (error_t (*fun)(struct node *))
diskfs_nrele (node);
}
+ free (node_list);
return err;
}
--
1.7.10.4
