Hi Charles,
I just realized I sent two messagens (with two patches) yesterday to
the wrong email address yesterday. Grumble, grumble....
Anyways, here is one....
On Wednesday 02 December 2009 12:58:52 Charles Manning wrote:
>
> What do you mean by "no longer mountable"?
>
here is the console output:
# echo 0x2018 > /sys/module/yaffs/parameters/yaffs_traceMask
# dmesg -n 8
# mount -t yaffs2 /dev/mtdblock6 /mnt/nand_var/
yaffs: dev is 32505862 name is "mtdblock6"
yaffs: passed flags ""
yaffs: Attempting MTD mount on 31.6, "mtdblock6"
yaffs_ScanBackwards starts intstartblk 1 intendblk 896...
Block scanning block 1 state 2 seq 4097
Block scanning block 2 state 2 seq 4098
Block scanning block 3 state 3 seq 0
Block empty
Block scanning block 4 state 2 seq 4209
=== many similar messages deleted ===
Block scanning block 896 state 3 seq 0
Block empty
123 blocks to be sorted...
...done
123 blocks to be scanned
Allocating from 142 63
Allocating from 142 62
=== many similar messages deleted ===
Allocating from 142 3
Allocating from 142 2
mount: wrong fs type, bad option, bad superblock on /dev/mtdblock6,
missing codepage or other error
In some cases useful info is found in syslog - try
dmesg | tail or so
And then the fs is not mounted.
So, after analyzing what is happening, I determined that it made it to here:
yaffs_guts.c:6720
case YAFFS_OBJECT_TYPE_SYMLINK:
if (oh) {
in->variant.symLinkVariant.alias = yaffs_CloneString(oh->alias);
if (!in->variant.symLinkVariant.alias)
alloc_failed = 1;
}
break;
I modified it to print out oh->alias, and also oh->name. oh->alias is
the empty string, which causes yaffs_CloneString(oh->alias) to return
NULL. oh->name had value "deleted". This what led me to discover the
issue.
Later On Wednesday 02 December 2009 Charles Manning wrote:
> Thanks David
>
You can call me Chris (my first name).
> There is definitely a problem if you unlink a symlink.
>
> I am a bit concerned about the apporach you propose because I don't like the
> idea of using the object after it has been deleted.
>
> Instead I propose to do something like:
> static int yaffs_DeleteSymLink(yaffs_Object * in)
> {
> YFREE(in->variant.symLinkVariant.alias);
> + in->variant.symLinkVariant = NULL;
> return yaffs_DoGenericObjectDeletion(in);
> }
>
> then handling a NULL value properly where need be.
>
> -- Charles
>
Thanks for taking the time to review this. I see your point. I think
you mean:
+ in->variant.symLinkVariant.alias = NULL;
^^^^^
So, I went down this path and below is the patch. (note this one
should apply successfully on CVS, unlike my last patch...)
I took care to allow mounting a filesystem in the condition my
"unmountable filesystem" is currently in. I left this comment in the
code in a couple spots to prevent others from changing the critical
part.
/* Note there used to be a bug where for
* deleted symlinks, .alias was copied
* from after a free. Therefore, the
* value of oh->alias must not be
* depended upon for deleted symlinks.
*/
I'm not sure I like this patch. It will work, but it seems to have
added many tests. I have tested this a little. I plan to test it
more. One thing I cannot easily test is the YAFFS1 case.
yaffs_CloneString() does not differentiate between being passed an empty
string to copy, and running out of memory. I was afraid to change it.
Perhaps that approach would have resulted in a smaller patch.
Is it important that a deleted symlink goes into the dev->deletedDir?
--- yaffs2.orig/yaffs_guts.c 2009-11-10 17:40:41.000000000 -0800
+++ yaffs2.fixed7/yaffs_guts.c 2009-12-02 11:23:36.000000000 -0800
@@ -3803,10 +3803,14 @@
/* Do nothing */
break;
case YAFFS_OBJECT_TYPE_SYMLINK:
- yaffs_strncpy(oh->alias,
- in->variant.symLinkVariant.alias,
- YAFFS_MAX_ALIAS_LENGTH);
- oh->alias[YAFFS_MAX_ALIAS_LENGTH] = 0;
+ if (! in->variant.symLinkVariant.alias)
+ oh->alias[0] = 0;
+ else {
+ yaffs_strncpy(oh->alias,
+ in->variant.symLinkVariant.alias,
+ YAFFS_MAX_ALIAS_LENGTH);
+ oh->alias[YAFFS_MAX_ALIAS_LENGTH] = 0;
+ }
break;
}
@@ -5089,6 +5093,8 @@
case YAFFS_OBJECT_TYPE_FILE:
return obj->variant.fileVariant.fileSize;
case YAFFS_OBJECT_TYPE_SYMLINK:
+ if (!obj->variant.symLinkVariant.alias)
+ return 0;
return yaffs_strlen(obj->variant.symLinkVariant.alias);
default:
return 0;
@@ -5236,6 +5242,7 @@
static int yaffs_DeleteSymLink(yaffs_Object *in)
{
YFREE(in->variant.symLinkVariant.alias);
+ in->variant.symLinkVariant.alias = NULL;
return yaffs_DoGenericObjectDeletion(in);
}
@@ -5974,10 +5981,20 @@
/* Do nothing */
break;
case YAFFS_OBJECT_TYPE_SYMLINK:
- in->variant.symLinkVariant.alias =
- yaffs_CloneString(oh->alias);
- if (!in->variant.symLinkVariant.alias)
- alloc_failed = 1;
+ /* Note there used to be a bug where for
+ * deleted symlinks, .alias was copied
+ * from after a free. Therefore, the
+ * value of oh->alias must not be
+ * depended upon for deleted symlinks.
+ */
+ if (oh->parentObjectId == YAFFS_OBJECTID_DELETED)
+ in->variant.symLinkVariant.alias = NULL;
+ else {
+ in->variant.symLinkVariant.alias =
+ yaffs_CloneString(oh->alias);
+ if (!in->variant.symLinkVariant.alias)
+ alloc_failed = 1;
+ }
break;
}
@@ -6098,10 +6115,14 @@
yaffs_SetObjectName(in, oh->name);
if (in->variantType == YAFFS_OBJECT_TYPE_SYMLINK) {
- in->variant.symLinkVariant.alias =
- yaffs_CloneString(oh->alias);
- if (!in->variant.symLinkVariant.alias)
- alloc_failed = 1; /* Not returned to caller */
+ if (oh->parentObjectId == YAFFS_OBJECTID_DELETED)
+ in->variant.symLinkVariant.alias = NULL;
+ else {
+ in->variant.symLinkVariant.alias =
+ yaffs_CloneString(oh->alias);
+ if (!in->variant.symLinkVariant.alias)
+ alloc_failed = 1; /* Not reted to be a bug where for
+ * deleted symlinks, .alias was copied
+ * from after a free. Therefore, the
+ * value of oh->alias must not be
+ * depended upon for deleted symlinks.
+ */
+
+ if (oh && oh->parentObjectId == YAFFS_OBJECTID_DELETED)
+ in->variant.symLinkVariant.alias = NULL;
+
(text/plain)