This morning we discovered that ramfs started failing. There is something
you don't run into every day...

Could I have some comments on the attached patch?

The problem is that it is illegal to cache pointers returned from
findbuffer_node()
across findbuffer_node() calls, due to reallocing happening, ANAICT.

The patch closes a *tiny* window where the pointer can change. With longer
filenames it is more likely that this can happen, but I suppose
shorter filenames
are not immune to these problems...

Thanks to Laurentiu Cocanu([email protected]) for spotting this!

He has a magic computer that happened to build a romfs image which was copied
to ramfs where the order of the files just so happened to trigger this
problem :-)

-- 
Øyvind Harboe

Visit us at Embedded World, March 2nd-4th. IS2T's stand, HALL 10 - 118
http://www.zylin.com/events_embeddedworld.html

US toll free 1-866-980-3434 / International +47 51 63 25 00
http://www.zylin.com/zy1000.html
ARM7 ARM9 ARM11 XScale Cortex
JTAG debugger and flash programmer
? fixramfs.txt
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/ram/current/ChangeLog,v
retrieving revision 1.22
diff -w -u -5 -p -r1.22 ChangeLog
--- ChangeLog   28 Apr 2009 13:06:11 -0000      1.22
+++ ChangeLog   22 Feb 2010 14:31:39 -0000
@@ -1,5 +1,11 @@
+2010-02-22  Oyvind Harboe  <[email protected]>
+
+       src/ramfs.c: Fix directory file name corruption.
+       Do not cache pointers returned from findbuffer_node(),
+       but do a fresh lookup based on position.
+
 2009-04-28  John Dallaway  <[email protected]>
 
        cdl/ramfs.cdl: Use CYGPKG_IO_FILEIO as the parent.
 
 2008-04-02  Xinghua Yang <[email protected]>
Index: src/ramfs.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/fs/ram/current/src/ramfs.c,v
retrieving revision 1.12
diff -w -u -5 -p -r1.12 ramfs.c
--- src/ramfs.c 29 Jan 2009 17:48:54 -0000      1.12
+++ src/ramfs.c 22 Feb 2010 14:31:40 -0000
@@ -1120,11 +1120,12 @@ static int add_direntry( ramfs_node *dir
                          int namelen,           // length of name
                          ramfs_node *node       // node to reference
                        )
 {
     off_t pos = 0;
-    ramfs_dirent *d = NULL, *dp = NULL;
+    off_t prev_pos = 0;
+    ramfs_dirent *d = NULL, *dp;
     cyg_bool isfirst = true;
 
     // Loop inserting fragments of the name into the directory until we
     // have found a home for them all.
     
@@ -1152,10 +1153,25 @@ static int add_direntry( ramfs_node *dir
             }
 
             break;
         }
 
+        /* Tricky! here we have to look up the previous segment as reallocating
+         * could have moved it. */
+        if (!isfirst)
+        {
+            cyg_uint8 *buf;
+            size_t size;
+            int err = findbuffer_node( dir, prev_pos, &buf, &size, false );
+            if( err != ENOERR ) return err;
+
+            dp = (ramfs_dirent *)buf;
+        } else
+        {
+               dp = NULL;
+        }
+
         // d now points to a free dirent structure
 
         d->node         = node;
         d->inuse        = 1;
         d->first        = isfirst;
@@ -1165,10 +1181,11 @@ static int add_direntry( ramfs_node *dir
 
         memcpy( d->name, name, fraglen );
 
         name            += fraglen;
         namelen         -= fraglen;
+        prev_pos               = pos;
         pos             += sizeof(ramfs_dirent);
         dp              = d;
         isfirst         = false;
         
     }

Reply via email to