Patchwork JFFS2 corruption when mounting filesystem with filenames oflength> 7

login
register
mail settings
Submitter Steve Deiters
Date June 25, 2010, 11:48 p.m.
Message ID <181804936ABC2349BE503168465576460F20CB90@exchserver.basler.com>
Download mbox | patch
Permalink /patch/57041/
State New
Headers show

Comments

Steve Deiters - June 25, 2010, 11:48 p.m.
> -----Original Message-----
> From: linux-mtd-bounces@lists.infradead.org 
> [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of 
> Steve Deiters
> Sent: Thursday, June 24, 2010 3:02 PM
> To: linux-mtd@lists.infradead.org
> Subject: RE: JFFS2 corruption when mounting filesystem with 
> filenames oflength> 7
> 
> > -----Original Message-----
> > From: linux-mtd-bounces@lists.infradead.org
> > [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of Steve 
> > Deiters
> > Sent: Wednesday, June 23, 2010 5:42 PM
> > To: linux-mtd@lists.infradead.org
> > Subject: RE: JFFS2 corruption when mounting filesystem with 
> filenames 
> > oflength > 7
> > 
> > > -----Original Message-----
> > > From: linux-mtd-bounces@lists.infradead.org
> > > [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of Steve 
> > > Deiters
> > > Sent: Wednesday, June 23, 2010 5:21 PM
> > > To: linux-mtd@lists.infradead.org
> > > Subject: JFFS2 corruption when mounting filesystem with
> > filenames of
> > > length > 7
> > > 
> > > I found an archived post which seems to be identical to my issue.
> > > However, this is quite old and there never seemed to be any 
> > > resolution.
> > > 
> > > http://www.infradead.org/pipermail/linux-mtd/2006-September/01
> > > 6491.html
> > > 
> > > If I mount a filesystem that has filenames greater than 7
> > characters
> > > in length, the files are corrupted when I mount.
> > > In my case, I am making a
> > > JFFS2 image with mkfs.jffs2 and flashing it in with u-boot.  
> > > However, I have attached a workflow where I erase the Flash
> > and create
> > > a new filesystem completely within Linux and it gives the same 
> > > behavior.  I can list the files with the 'ls'
> > > command from within u-boot.  If I mount from within 
> Linux, and then 
> > > reboot into u-boot, it will not display any files that had
> > a filename
> > > greater than 7 characters.
> > > 
> > > I enabled the MTD debug verbosity at level 2 for the
> > attached example
> > > session.
> > > 
> > > I am running on a custom board with a MPC5121 and Linux 2.6.33.4.
> > > 
> > > Thanks in advance for any help.
> > 
> > 
> > Sorry for the jumbled mess.  Looks like the line endings are messed 
> > up.
> > Trying again.  I also provided this as an attachment in 
> case it gets 
> > messed up again.
> 
> Once again sorry for the mess.
> 
> I tried this again with the DENX-v2.6.34 tag in the DENX git 
> repository (git://git.denx.de/linux-2.6-denx.git).  The only 
> modification I made was to add my dts file.  I still get the 
> same issue I had before.
> 
> I've attached my kernel config if that gives any clues.
> 
> Are there any thoughts on what may be causing this?
> 
> Thanks.


I think there may be something weird going on with the memcpy in my
build.  If I use the following patch I no longer get errors when I mount
the filesystem.  All I did was replace the memcpy with a loop.

I'm not sure what's special about this particular use of memcpy.  I
can't believe that things would be working as well as they do if memcpy
was broken in general.

This is on a PowerPC 32 bit build for a MPC5121.  I am using a GCC 4.1.2
to compile.  Is anyone aware of any issues with memcpy in this
configuration?

Thanks.

-------

 
 	crc = crc32(0, fd->name, rd->nsize);
Joakim Tjernlund - June 26, 2010, 1:27 p.m.
>
> > -----Original Message-----
> > From: linux-mtd-bounces@lists.infradead.org
> > [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of
> > Steve Deiters
> > Sent: Thursday, June 24, 2010 3:02 PM
> > To: linux-mtd@lists.infradead.org
> > Subject: RE: JFFS2 corruption when mounting filesystem with
> > filenames oflength> 7
> >
> > > -----Original Message-----
> > > From: linux-mtd-bounces@lists.infradead.org
> > > [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of Steve
> > > Deiters
> > > Sent: Wednesday, June 23, 2010 5:42 PM
> > > To: linux-mtd@lists.infradead.org
> > > Subject: RE: JFFS2 corruption when mounting filesystem with
> > filenames
> > > oflength > 7
> > >
> > > > -----Original Message-----
> > > > From: linux-mtd-bounces@lists.infradead.org
> > > > [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of Steve
> > > > Deiters
> > > > Sent: Wednesday, June 23, 2010 5:21 PM
> > > > To: linux-mtd@lists.infradead.org
> > > > Subject: JFFS2 corruption when mounting filesystem with
> > > filenames of
> > > > length > 7
> > > >
> > > > I found an archived post which seems to be identical to my issue.
> > > > However, this is quite old and there never seemed to be any
> > > > resolution.
> > > >
> > > > http://www.infradead.org/pipermail/linux-mtd/2006-September/01
> > > > 6491.html
> > > >
> > > > If I mount a filesystem that has filenames greater than 7
> > > characters
> > > > in length, the files are corrupted when I mount.
> > > > In my case, I am making a
> > > > JFFS2 image with mkfs.jffs2 and flashing it in with u-boot.
> > > > However, I have attached a workflow where I erase the Flash
> > > and create
> > > > a new filesystem completely within Linux and it gives the same
> > > > behavior.  I can list the files with the 'ls'
> > > > command from within u-boot.  If I mount from within
> > Linux, and then
> > > > reboot into u-boot, it will not display any files that had
> > > a filename
> > > > greater than 7 characters.
> > > >
> > > > I enabled the MTD debug verbosity at level 2 for the
> > > attached example
> > > > session.
> > > >
> > > > I am running on a custom board with a MPC5121 and Linux 2.6.33.4.
> > > >
> > > > Thanks in advance for any help.
> > >
> > >
> > > Sorry for the jumbled mess.  Looks like the line endings are messed
> > > up.
> > > Trying again.  I also provided this as an attachment in
> > case it gets
> > > messed up again.
> >
> > Once again sorry for the mess.
> >
> > I tried this again with the DENX-v2.6.34 tag in the DENX git
> > repository (git://git.denx.de/linux-2.6-denx.git).  The only
> > modification I made was to add my dts file.  I still get the
> > same issue I had before.
> >
> > I've attached my kernel config if that gives any clues.
> >
> > Are there any thoughts on what may be causing this?
> >
> > Thanks.
>
>
> I think there may be something weird going on with the memcpy in my
> build.  If I use the following patch I no longer get errors when I mount
> the filesystem.  All I did was replace the memcpy with a loop.
>
> I'm not sure what's special about this particular use of memcpy.  I
> can't believe that things would be working as well as they do if memcpy
> was broken in general.
>
> This is on a PowerPC 32 bit build for a MPC5121.  I am using a GCC 4.1.2
> to compile.  Is anyone aware of any issues with memcpy in this
> configuration?
>
> Thanks.
>
> -------
>
> diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
> index 46f870d..673caa2 100644
> --- a/fs/jffs2/scan.c
> +++ b/fs/jffs2/scan.c
> @@ -1038,7 +1038,10 @@ static int jffs2_scan_dirent_node(struct
> jffs2_sb_info *c, struct jffs2_eraseblo
>     if (!fd) {
>        return -ENOMEM;
>     }
> -   memcpy(&fd->name, rd->name, checkedlen);

Are the pointers to memcpy overlapping? If so memcpy is undefined
and you have to use memmove().

> +   int i;
> +   for(i = 0; i < checkedlen; i++)
> +      ((unsigned char*)fd->name)[i] = ((const unsigned
> char*)rd->name)[i];
> +
>     fd->name[checkedlen] = 0;
>
>     crc = crc32(0, fd->name, rd->nsize);
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
Steve Deiters - June 28, 2010, 9:25 p.m.
> I think there may be something weird going on with the memcpy 
> in my build.  If I use the following patch I no longer get 
> errors when I mount the filesystem.  All I did was replace 
> the memcpy with a loop.
> 
> I'm not sure what's special about this particular use of 
> memcpy.  I can't believe that things would be working as well 
> as they do if memcpy was broken in general.
> 
> This is on a PowerPC 32 bit build for a MPC5121.  I am using 
> a GCC 4.1.2 to compile.  Is anyone aware of any issues with 
> memcpy in this configuration?
> 
> Thanks.

It seems this processor is mangling the data when it access unaligned
addresses into Flash with a lwz instruction.  The memcpy implementation
in copy_32.S aligns the destination, leaving the source possibly
unaligned.  In this particular instance the source is an address into my
Flash address space which is causing the problem.  When the filenames
were < 8 it always does a bytewise copy which is why I wasn't having
problems with those.

It seems this only occurs when I have the translation enabled.  If I
clear the DR bit in the MSR and then repeat the instruction with the
corresponding physical address it will read correctly.

I'm not sure if this is expected behavior with this core.  I can fix at
least this one problem by using memcpy_fromio since it does alignment
checks for the source and destination.

I'm not sure what the proper fix is for this.  If I use memcpy_fromio I
think I'll just run into problems somewhere else.  Any other
suggestions?

Thanks.
Wolfgang Denk - June 28, 2010, 10:30 p.m.
Dear "Steve Deiters",

In message <181804936ABC2349BE503168465576460F20D651@exchserver.basler.com> you wrote:
> > I think there may be something weird going on with the memcpy 
> > in my build.  If I use the following patch I no longer get 
> > errors when I mount the filesystem.  All I did was replace 
> > the memcpy with a loop.
> > 
> > I'm not sure what's special about this particular use of 
> > memcpy.  I can't believe that things would be working as well 
> > as they do if memcpy was broken in general.
> > 
> > This is on a PowerPC 32 bit build for a MPC5121.  I am using 
> > a GCC 4.1.2 to compile.  Is anyone aware of any issues with 
> > memcpy in this configuration?
> > 
> > Thanks.
> 
> It seems this processor is mangling the data when it access unaligned
> addresses into Flash with a lwz instruction.  The memcpy implementation
> in copy_32.S aligns the destination, leaving the source possibly
> unaligned.  In this particular instance the source is an address into my
> Flash address space which is causing the problem.  When the filenames
> were < 8 it always does a bytewise copy which is why I wasn't having
> problems with those.
> 
> It seems this only occurs when I have the translation enabled.  If I
> clear the DR bit in the MSR and then repeat the instruction with the
> corresponding physical address it will read correctly.
> 
> I'm not sure if this is expected behavior with this core.  I can fix at
> least this one problem by using memcpy_fromio since it does alignment
> checks for the source and destination.

Both the MPC52xx and MPC512x have know problems (silent data
corruption) with unaligned accesses on the local bus.

This can be trivially demonstrated in U-Boot by reading the flash
memory with 32 bit accesses; for example like this:

=> md FC0C0000 20
fc0c0000: 7012ab65 01616464 636f6e73 3d736574    p..e.addcons=set
fc0c0010: 656e7620 626f6f74 61726773 20247b62    env bootargs ${b
fc0c0020: 6f6f7461 7267737d 20636f6e 736f6c65    ootargs} console
fc0c0030: 3d247b63 6f6e736f 6c657d2c 247b6261    =${console},${ba
fc0c0040: 75647261 74657d00 61646469 703d7365    udrate}.addip=se
fc0c0050: 74656e76 20626f6f 74617267 7320247b    tenv bootargs ${
fc0c0060: 626f6f74 61726773 7d206970 3d247b69    bootargs} ip=${i
fc0c0070: 70616464 727d3a24 7b736572 76657269    paddr}:${serveri
=> md FC0C0001 20
fc0c0001: 65726901 00000063 0000003d 00000065    eri....c...=...e
fc0c0011: 00000062 00000061 00000020 0000006f    ...b...a... ...o
fc0c0021: 00000072 00000020 00000073 0000003d    ...r... ...s...=
fc0c0031: 0000006f 0000006c 00000024 00000075    ...o...l...$...u
fc0c0041: 00000074 00000061 00000070 00000074    ...t...a...p...t
fc0c0051: 00000020 00000074 00000073 00000062    ... ...t...s...b
fc0c0061: 00000061 0000007d 0000003d 00000070    ...a...}...=...p
fc0c0071: 00000072 0000007b 00000076 00000070    ...r...{...v...p

> I'm not sure what the proper fix is for this.  If I use memcpy_fromio I
> think I'll just run into problems somewhere else.  Any other
> suggestions?

See http://article.gmane.org/gmane.comp.boot-loaders.u-boot/80278 for
how we fix this in U-Boot.

Best regards,

Wolfgang Denk

Patch

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index 46f870d..673caa2 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -1038,7 +1038,10 @@  static int jffs2_scan_dirent_node(struct
jffs2_sb_info *c, struct jffs2_eraseblo
 	if (!fd) {
 		return -ENOMEM;
 	}
-	memcpy(&fd->name, rd->name, checkedlen);
+	int i;
+	for(i = 0; i < checkedlen; i++)
+		((unsigned char*)fd->name)[i] = ((const unsigned
char*)rd->name)[i];
+
 	fd->name[checkedlen] = 0;