Skip to contentSkip to conversation

Patch (repost): cramfs memory corruption fix

PATCH In review Adam J. Richter 11 participants 22 messages Jan 7, 2001 – Jan 11, 2001

Subsystems (2)
lore
Share & tools
lore
#0 lore raw
Quoted 11 lines

From: "David L. Parsley" <[email protected]>

Using root=/dev/ram0 and a cramfs initrd gives me 'wrong magic' when it tries to boot. Even more bizarre, if cramfs is compiled in the kernel when I use a romfs root, it says 'wrong magic' then mounts the romfs but can't find init. If I take cramfs out of the kernel, the romfs mounts & init runs fine. I just saw this with ac3.

ramfs croaks with 'kernel BUG in filemap.c line 2559' anytime I make a file in ac2 and ac3. Works fine in 2.4.0 vanilla. Should be quite repeatable...

This sounds like a bug that I posted a fix for a long time ago.
cramfs calls bforget on the superblock area, destroying that block of the ramdisk, even when the ramdisk does not contain a cramfs file system. Normally, bforget is called on block that really can be trashed, such as blocks release by truncate or unlink. If it worked for you before, you were just getting lucky. Here is the patch.

Linus, please consider applying this. Thank you.

By the way, the other approach to fixing this problem would
be to change bforget not to trash blocks marked with BH_Protected (I think that is just ramdisk blocks), but that would waste memory, because we really can release blocks from things like truncating or unlinking files.

Signature · 4 lines
Adam J. Richter     __     ______________   4880 Stevens Creek Blvd, Suite 104
[email protected]     \ /                  San Jose, California 95129-1034
+1 408 261-6630         | g g d r a s i l   United States of America
fax +1 408 261-6631      "Free Software For The Rest Of Us."
fs/cramfs/inode.c renamed from tmp/adam/linux-2.4.0/fs/cramfs/inode.c +1 -1
@@ -138,7 +138,7 @@
 		struct buffer_head * bh = bh_array[i];
 		if (bh) {
 			memcpy(data, bh->b_data, PAGE_CACHE_SIZE);
-			bforget(bh);
+			brelse(bh);
 		} else
 			memset(data, 0, PAGE_CACHE_SIZE);
 		data += PAGE_CACHE_SIZE;
#1 lore raw
Quoted 3 lines
Quoted 3 lines

ramfs croaks with 'kernel BUG in filemap.c line 2559' anytime I make a file in ac2 and ac3. Works fine in 2.4.0 vanilla. Should be quite repeatable...

I'll take a look at the ramfs one. I may have broken something else when fixing everything else with ramfs (like unlink) crashing

Quoted 8 lines

This sounds like a bug that I posted a fix for a long time ago.
cramfs calls bforget on the superblock area, destroying that block of the ramdisk, even when the ramdisk does not contain a cramfs file system. Normally, bforget is called on block that really can be trashed, such as blocks release by truncate or unlink. If it worked for you before, you were just getting lucky. Here is the patch.

Linus, please consider applying this. Thank you.

This isnt the fix. If -ac also fails well it contains this cramfs fix. So there must be other problems

Alan

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#2 lore raw
Quoted 6 lines · Alan Cox
Quoted 3 lines
Quoted 3 lines

ramfs croaks with 'kernel BUG in filemap.c line 2559' anytime I make a file in ac2 and ac3. Works fine in 2.4.0 vanilla. Should be quite repeatable...

I'll take a look at the ramfs one. I may have broken something else when fixing everything else with ramfs (like unlink) crashing

Ehh.. Plain 2.4.0 ramfs is fine, assuming you add a "UnlockPage()" to ramfs_writepage(). So what do you mean by "fixing everything else"?

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw
Quoted 5 lines
Quoted 2 lines

I'll take a look at the ramfs one. I may have broken something else when fixing everything else with ramfs (like unlink) crashing

Ehh.. Plain 2.4.0 ramfs is fine, assuming you add a "UnlockPage()" to ramfs_writepage(). So what do you mean by "fixing everything else"?

-ac has the rather extended ramfs with resource limits and stuff. That one also has rather more extended bugs 8). AFAIK none of those are in the vanilla ramfs code

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#4 lore raw
Quoted 9 lines · Alan Cox
Quoted 5 lines
Quoted 2 lines

I'll take a look at the ramfs one. I may have broken something else when fixing everything else with ramfs (like unlink) crashing

Ehh.. Plain 2.4.0 ramfs is fine, assuming you add a "UnlockPage()" to ramfs_writepage(). So what do you mean by "fixing everything else"?

-ac has the rather extended ramfs with resource limits and stuff. That one also has rather more extended bugs 8). AFAIK none of those are in the vanilla ramfs code

Ahh, ok.

This is actually where I agree with whoever it was that said that ramfs as it stands now (without the limit checking etc) is much nicer simply because it can act as an example of how to do a simple filesystem.

I wonder what to do about this - the limits are obviously useful, as would the "use swap-space as a backing store" thing be. At the same time I'd really hate to lose the lean-mean-clean ramfs.

Anyway, I'm quite sure that the resource stuff would not be 2.4.1 material (adding the UnlockPage() is), so I won't have to worry about this issue for a while.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#5 lore raw
Quoted 6 lines · Linus Torvalds
Quoted 3 lines · Alan Cox

-ac has the rather extended ramfs with resource limits and stuff. That one also has rather more extended bugs 8). AFAIK none of those are in the vanilla ramfs code

Quoted 7 lines

This is actually where I agree with whoever it was that said that ramfs as it stands now (without the limit checking etc) is much nicer simply because it can act as an example of how to do a simple filesystem.

I wonder what to do about this - the limits are obviously useful, as would the "use swap-space as a backing store" thing be. At the same time I'd really hate to lose the lean-mean-clean ramfs.

Sounds like a job for ... <drum roll> ... tmpfs!!

(and yes, I share your opinion that ramfs is nice _because_ it's an easy example for filesystem code teaching)

cheers,

Rik
--
Virtual memory is like a game you can't win; However, without VM there's truly nothing to lose...

		http://www.surriel.com/

http://www.conectiva.com/ http://distro.conectiva.com.br/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#6 lore raw
Quoted 4 lines

Sounds like a job for ... <drum roll> ... tmpfs!!

(and yes, I share your opinion that ramfs is nice _because_ it's an easy example for filesystem code teaching)

The resource tracking ramfs isnt that much uglier to be honest. One that went off using backing store would be, but ramfs with limits simply ensures that

dd if=/dev/zero of=/mnt/ram/foo

doesnt crash your box
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#7 lore raw
In reply to Rik van Riel
Quoted 13 lines · Rik van Riel
Quoted 4 lines · Linus Torvalds
Quoted 2 lines · Alan Cox

-ac has the rather extended ramfs with resource limits and stuff. That one also has rather more extended bugs 8). AFAIK none of those are in the

vanilla

Quoted 6 lines
Quoted 1 line

ramfs code

This is actually where I agree with whoever it was that said that ramfs as it stands now (without the limit checking etc) is much nicer simply because it can act as an example of how to do a simple filesystem.

Quoted 5 lines
Quoted 3 lines

I wonder what to do about this - the limits are obviously useful, as would the "use swap-space as a backing store" thing be. At the same time I'd really hate to lose the lean-mean-clean ramfs.

Sounds like a job for ... <drum roll> ... tmpfs!!

If you need tmpfs the VFS layer is broken. For 99% of everything performance is determined by VFS layer caching. A fs that uses swap space as a backing store is not a big win. You just have a fs that doesn't support sync and you can add a mount option to a normal fs if you want that.

I've written the filesystem and it was a dumb idea.

Ramfs with (maybe) some basic limits has a place. tmpfs is just extra code to maintain.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#8 lore raw
In reply to Linus Torvalds

Hi Linus,

Quoted 3 lines · Linus Torvalds

I wonder what to do about this - the limits are obviously useful, as would the "use swap-space as a backing store" thing be. At the same time I'd really hate to lose the lean-mean-clean ramfs.

Let me repeat on this issue: shmem.c has everything needed for this despite read and write and they should be really easy to add.

I did not plan to write them in the near future because I did not think that this is a really wanted feature. But I can look into it.

Greetings
Christoph

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#9 lore raw
Quoted 7 lines · In article <[email protected]> you wrote:

Hi Linus,

Quoted 3 lines · Linus Torvalds

I wonder what to do about this - the limits are obviously useful, as would the "use swap-space as a backing store" thing be. At the same time I'd really hate to lose the lean-mean-clean ramfs.

Quoted 6 lines

Let me repeat on this issue: shmem.c has everything needed for this despite read and write and they should be really easy to add.

I did not plan to write them in the near future because I did not think that this is a really wanted feature. But I can look into it.

I had a prototype tmpfs in -test10 (ro so) times. It based on ramfs for all the metadata stuff and used the (old) shmfs code for swap-backed data. The only real problem the code had, was that it needed a ->allocpage address_space method in place of page_cache_alloc() to directly swap-in pages in ->read. IF anyone is interested I could forward port it to 2.4.0 and the new shmfs.

Christoph

Signature · 5 lines
Whip me.  Beat me.  Make me maintain AIX.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#10 lore raw

Hi Christoph,

Quoted 6 lines · Christoph Hellwig

I had a prototype tmpfs in -test10 (ro so) times. It based on ramfs for all the metadata stuff and used the (old) shmfs code for swap-backed data. The only real problem the code had, was that it needed a ->allocpage address_space method in place of page_cache_alloc() to directly swap-in pages in ->read. IF anyone is interested I could forward port it to 2.4.0 and the new shmfs.

Be aware that there is nothing left from the old shm memory/swap handling in 2.4.0. It is a copy of ramfs plus additions for swap and resource limits

If you would be willing to add the read and write functions to the new coding I would be very happy.

Greetings
Christoph

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#11 lore raw
In reply to Christoph Rohland
Quoted 10 lines · Linus Torvalds
Quoted 3 lines

I wonder what to do about this - the limits are obviously useful, as would the "use swap-space as a backing store" thing be. At the same time I'd really hate to lose the lean-mean-clean ramfs.

Let me repeat on this issue: shmem.c has everything needed for this despite read and write and they should be really easy to add.

I did not plan to write them in the near future because I did not think that this is a really wanted feature. But I can look into it.

I have been thinking about this. I think we should merge the size limiting code with the example clean ramfs code. Having spent a while debugging the LFS checks and some other funnies I realised one problem with the ramfs in 2.4.0 as an example. It does not demonstrate error cases, which the new one does.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#12 lore raw

Hi Alan,

Quoted 5 lines · Alan Cox

I have been thinking about this. I think we should merge the size limiting code with the example clean ramfs code. Having spent a while debugging the LFS checks and some other funnies I realised one problem with the ramfs in 2.4.0 as an example. It does not demonstrate error cases, which the new one does.

For demonstration purposes perhaps. But I do not see a lot of value of using ramfs if shmem could do read and write.

Greetings
Christoph
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#13 lore raw
In reply to Alan Cox
Quoted 3 lines · Alan Cox

-ac has the rather extended ramfs with resource limits and stuff. That one also has rather more extended bugs 8). AFAIK none of those are in the vanilla ramfs code

Nifty stuff, too; it's nice for a ramfs mount to show up in 'df' with useful figures. Shame I can't put anything there. ;-)

2.4.0 ramfs with the one-liner does it's job for me already; what I'd really love to fool with is _cramfs_. ;-) In case you missed the beginning of this thread: all my cramfs initrd's fail to mount as /dev/ram0 with 'wrong magic'; their romfs counterparts work fine. I did manage to pivot_root into a cramfs root, but it blew up pretty quick with 'attempt to access beyond end of device', and killed my init shell. Then there's the wierdness where cramfs compiled in the kernel corrupts the romfs. Adam's one-liner (bforget -> brelse on superblock) didn't fix this.

The cramfs docs are contradictory btw; the kernel help says 'doesn't support hardlinks', and Documentation/filesystems/cramfs.txt says 'Hardlinks are supported, but...'. I made my cramfs with and without hardlinks (to busybox); but that didn't affect whether it mounted. I haven't tested whether this fixes the 'access beyond end of device' problems.

regards,
David

Signature · 7 lines
David L. Parsley
Network Administrator
Roanoke College
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#14 lore raw
Quoted 10 lines · David L. Parsley

2.4.0 ramfs with the one-liner does it's job for me already; what I'd really love to fool with is _cramfs_. ;-) In case you missed the beginning of this thread: all my cramfs initrd's fail to mount as /dev/ram0 with 'wrong magic'; their romfs counterparts work fine. I did manage to pivot_root into a cramfs root, but it blew up pretty quick with 'attempt to access beyond end of device', and killed my init shell. Then there's the wierdness where cramfs compiled in the kernel corrupts the romfs. Adam's one-liner (bforget -> brelse on superblock) didn't fix this.

Uhhuh. I've never used cramfs on a ramdisk, but I have a guess: cramfs _really_ wants to have 4kB blocks. I bet your ramdisk has 1kB blocks (the default).

Have you tried using

	ramdisk_blocksize=4096

as a boot option?

Quoted 6 lines

The cramfs docs are contradictory btw; the kernel help says 'doesn't support hardlinks', and Documentation/filesystems/cramfs.txt says 'Hardlinks are supported, but...'. I made my cramfs with and without hardlinks (to busybox); but that didn't affect whether it mounted. I haven't tested whether this fixes the 'access beyond end of device' problems.

The documentation is right. cramfs does not support hardlinks.

HOWEVER, cramfs gives you the _effect_ of hardlinks by having mkcramfs notice that two files are the same (whether hardlinked or not), and re-using the data block pointer. This gives you all the space savings of hard links, with a twist: you can actually have two separate files, with separate permissions and owners etc, and they will be "linked" in the data linking meaning if the contents are the same.

So you could think of the cramfs links as being a superset of hardlinks. Or a "corruption" of hardlinks. The file shows up as two different files as far as unix tools are concerned (nlink == 1, different inode numbers etc), but becasue of the data sharing they have the disk usage pattern of a hardlink.

The reason you don't have (and cannot have) _true_ hardlinks is obvious:
ramfs does not have inodes. It only has directory entries with enough information to fill in a linux inode structure. Kind of like FAT, in that respect.

This, btw, also means that it's counterproductive to try to save space on the source tree that is crammed by trying to find identical files and hardlinking them - mkcramfs will do the same thing regardless.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#15 lore raw
In reply to Alan Cox

On Sun, Jan 07, 2001 at 11:56:38AM -0800, Linus Torvalds wrote:

This is actually where I agree with whoever it was that said that ramfs as it stands now (without the limit checking etc) is much nicer simply because it can act as an example of how to do a simple filesystem.

That was me. ramfs is mimnimalist and if we must extend/bloat ramfs then I suggest we adding a backing store al la tmpfs or memfs and change it's name.

Don't get me wrong, I _like_ ramfs and want a backing store and perhaps limits (I use ramfs for /tmp and decided to put over a million files in there once as a test, kaboom), but I'm no guru and would love to see ramfs stay as is -- because _I_ can understand it. To me, that is worth a lot -- ramfs is only 8k, tiny to keep about as an example I should think.

I wonder what to do about this - the limits are obviously useful, as would the "use swap-space as a backing store" thing be. At the same time I'd really hate to lose the lean-mean-clean ramfs.

ramfs-> skeletonfs
ramfs+bloat -> vmfs

--cw

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#16 lore raw
In reply to Adam J. Richter
Quoted 6 lines · Adam J. Richter

This sounds like a bug that I posted a fix for a long time ago.
cramfs calls bforget on the superblock area, destroying that block of the ramdisk, even when the ramdisk does not contain a cramfs file system. Normally, bforget is called on block that really can be trashed, such as blocks release by truncate or unlink.

I'd really prefer just not letting bforget() touch BH_Protected buffers. bforget() is also used by other things than unlink/truncate: it's used by various partition codes etc, and it's used by the raid logic.

Now, nobody wants to use RAID on a ramdisk, but the fact is that "bforget()" does not mean "forget the contents of this buffer", but it really means "you can forget this cached copy even if it is dirty, we're not likely to need it in the near future and can read it back in".

Also, if you care about memory usage, you're likely to be much better off using ramfs rather than something like "ext2 on ramdisk". You won't get the double buffering.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#17 lore raw
Quoted 3 lines · [email protected] said:

Also, if you care about memory usage, you're likely to be much better
off using ramfs rather than something like "ext2 on ramdisk". You won't get the double buffering.

That'll be even more useful once we can completely configure out all support for block devices too.

--
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#18 lore raw
In reply to Linus Torvalds
Quoted 8 lines · David Woodhouse
Quoted 3 lines · [email protected] said:

Also, if you care about memory usage, you're likely to be much better
off using ramfs rather than something like "ext2 on ramdisk". You won't get the double buffering.

That'll be even more useful once we can completely configure out all support for block devices too.

While the topic is raised..., I've hacked up cramfs for linear addressing to kill the "double buffering" effiect. However as David mentions the block device support thing is an issue here. What is a reasonable way to allow a cramfs partition to access the device directly, like the patch that I wrote, and be picked up in a reasonable way by the init system?

(Current system does some non-cool things like find out it's cramfs from /dev/rom <also a patch>, and within cramfs never accesses that block device anymore..., it's sort of silly, and _not_ the right way to do it.)

Thanks,
Shane Nay.
(Patches referenced here can be found at:
ftp://ftp.agendacomputing.com/pub/agenda/testing/CES/patches/ , contributed by various authors: Rob Leslie, Brad Laronde, and myself. If you want the mkcramfs with xip, just ask.)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#19 lore raw
Quoted 6 lines · [email protected] said:

While the topic is raised..., I've hacked up cramfs for linear
addressing to kill the "double buffering" effiect. However as David mentions the block device support thing is an issue here. What is a reasonable way to allow a cramfs partition to access the device directly, like the patch that I wrote, and be picked up in a reasonable way by the init system?

So far, I just cheat. JFFS doesn't need a block device, but uses the minor# to determine which MTD device to use. You could use the same trick.

--
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#20 lore raw

PATCH 1/1 one-liner fix for bforget() honoring BH_Protected; was: Re: Patch (repost): cramfs memory corruption fix

In reply to Linus Torvalds
Quoted 9 lines · Linus Torvalds
Quoted 6 lines · Adam J. Richter

This sounds like a bug that I posted a fix for a long time ago.
cramfs calls bforget on the superblock area, destroying that block of the ramdisk, even when the ramdisk does not contain a cramfs file system. Normally, bforget is called on block that really can be trashed, such as blocks release by truncate or unlink.

Quoted 3 lines

I'd really prefer just not letting bforget() touch BH_Protected buffers. bforget() is also used by other things than unlink/truncate: it's used by various partition codes etc, and it's used by the raid logic.

Yup, I backed out Adam's one-liner in favor of the attached one-liner. Tested on 2.4.0, but should patch cleanly to just about anything. ;-)

BTW Linus - you were of course right on the cramfs wanting 4096 blocksize... but without this fix, that doesn't matter much. ;-)

regards,
David

Signature · 3 lines
David L. Parsley
Network Administrator
Roanoke College
fs/buffer.c +3 -1
@@ -1145,13 +1145,15 @@
  * free list if it can.. We can NOT free the buffer if:
  *  - there are other users of it
  *  - it is locked and thus can have active IO
+ *  - it is marked BH_Protected
  */
 void __bforget(struct buffer_head * buf)
 {
 	/* grab the lru lock here to block bdflush. */
 	spin_lock(&lru_list_lock);
 	write_lock(&hash_table_lock);
-	if (!atomic_dec_and_test(&buf->b_count) || buffer_locked(buf))
+	if (!atomic_dec_and_test(&buf->b_count) || buffer_locked(buf) || 
+	    buffer_protected(buf))
 		goto in_use;
 	__hash_unlink(buf);
 	remove_inode_queue(buf);
#21 lore raw

Re: [PATCH] one-liner fix for bforget() honoring BH_Protected; was: Re: Patch (repost): cramfs memory corruption fix

Quoted 6 lines · David L. Parsley

Yup, I backed out Adam's one-liner in favor of the attached one-liner. Tested on 2.4.0, but should patch cleanly to just about anything. ;-)

BTW Linus - you were of course right on the cramfs wanting 4096 blocksize... but without this fix, that doesn't matter much. ;-)

So everything works the way you want it now? Just checking that there aren't any outstanding issues..

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/