[PATCH]jffs2:bugfix for should not appeared directory hard link

liu.song11 at zte.com.cn liu.song11 at zte.com.cn
Mon Nov 10 23:12:52 PST 2014


Brian Norris <computersforpeace at gmail.com> wrote on 2014-11-05 19:45:00:
> 
> This only describes the problem, but it doesn't describe the fix at all.
> Please provide a proper commit description.
> 
> Also, your patch is very big and line-wrapped, so it's very hard to
> figure out what it's doing, and it won't apply to my git tree. Please
> check your mailer settings.
> 
> All this is addressed in Documentation/SubmittingPatches, which I
> suggest you read.
> 
> Regards,
> Brian

Hi, Brian

Thanks a lot for your attention and suggestions. I've checked the mail 
settings and changed wrap line character number to a suitable value
which was only 70 and had made everything looked in a mess. Follow the 
guide, patch had checked by checkpatch.pl, but there also had warnings
about line over 80 characters, I had no idea to fix that except remove the 
code indentations, but i'm afraid that would make codes hard to read.
However, if there are any suggestions to fix this I would try.

The following is the description about this patch.

The patch is focus on jffs2 with summary. We check deletion dirent's crc 
in "jffs2_sum_process_sum_data" so could make its ref flag REF_NORMAL.
After this process, all crc correct deletion dirents' ref flag are 
REF_NORMAL then could separate from other nodes(REF_UNCHECKED).
We use this flag to protect unlinked deletion dirent directly turned to 
obsoleted in "jffs2_build_remove_unlinked_inode", because deletion dirents
maybe still useful if it's obsoleted dirent remain in flash. These 
unlinked deletion dirents will be checked whether still useful in gc time.

These unlinked deletion dirents in "jffs2_garbage_collect_pass" will skip 
to check, because its "jffs2_inode_cache"'s pino_nlink is zero.
Then in "jffs2_garbage_collect_pass", unlinked deletion dirents' 
"ic->state" is INO_STATE_UNCHECKED, but we had checked the crc during 
scan. So we give 
its a new state which is INO_STATE_DELETION. If gc find current node's 
"ic->state" is INO_STATE_DELETION will know it's an unlinked deletion 
dirent,
then will check whether its still useful in "
jffs2_garbage_collect_unlinked_dirent" which is similar to "
jffs2_garbage_collect_deletion_dirent" but use
different parameters. If the unlinked deletion dirent is still useful, we 
copy it, else, we could safely mark it obsoleted.

In "jffs2_do_unlink", there also has the same problem, it will obsolete 
the unlinked directory's child dirents which include deletion dirent, so 
we
should also protect the deletion dirent.

The main purpose of this patch is to prevent unlinked deletion direct 
directly turn to obsolete and handle it during gc time. We had do many 
tests on this
patch and it worked properly, the directory hard link couldn't occured. We 
had also test the deletion dirents' crc in scan time whether cost a lot,
the result is check 81809 deletion dirents' crc could within one second, 
so it will not extend the mounting time.

Thanks
Best Regards
Liu Song
-------------------------------------------------------------------------------------------------------------------------------------------------------------
diff --git a/build.c b/build.c
index a3750f9..ad7b8a6 100644
--- a/build.c
+++ b/build.c
@@ -205,7 +205,9 @@ static void jffs2_build_remove_unlinked_inode(struct 
jffs2_sb_info *c,
        while (raw != (void *)ic) {
                struct jffs2_raw_node_ref *next = raw->next_in_ino;
                dbg_fsbuild("obsoleting node at 0x%08x\n", 
ref_offset(raw));
-               jffs2_mark_node_obsolete(c, raw);
+               /* prevent make deletion dirent obsolete directly*/
+               if (ref_flags(raw) != REF_NORMAL)
+                       jffs2_mark_node_obsolete(c, raw);
                raw = next;
        }
 
diff --git a/fs.c b/fs.c
index 601afd1..ac3abe0 100644
--- a/fs.c
+++ b/fs.c
@@ -657,6 +657,7 @@ struct jffs2_inode_info *jffs2_gc_fetch_inode(struct 
jffs2_sb_info *c,
                                          ic->ino, ic->state);
                                sleep_on_spinunlock(&c->inocache_wq, 
&c->inocache_lock);
                        } else {
+                               ic->state = INO_STATE_DELETION;
                                spin_unlock(&c->inocache_lock);
                        }
 
diff --git a/gc.c b/gc.c
index 5a2dec2..3a0f84a 100644
--- a/gc.c
+++ b/gc.c
@@ -39,6 +39,14 @@ static int jffs2_garbage_collect_dnode(struct 
jffs2_sb_info *c, struct jffs2_era
                                       uint32_t start, uint32_t end);
 static int jffs2_garbage_collect_live(struct jffs2_sb_info *c,  struct 
jffs2_eraseblock *jeb,
                               struct jffs2_raw_node_ref *raw, struct 
jffs2_inode_info *f);
+static int jffs2_garbage_collect_unlinked_deletion(struct jffs2_sb_info 
*c,
+                                       struct jffs2_inode_cache *ic,
+                                       struct jffs2_raw_node_ref *raw,
+                                       struct jffs2_raw_dirent *rd);
+static int jffs2_garbage_collect_unlinked_dirent(struct jffs2_sb_info *c,
+                                       struct jffs2_eraseblock *jeb,
+                                       struct jffs2_raw_node_ref 
*raw_suspect,
+                                       struct jffs2_inode_cache *ic);
 
 /* Called with erase_completion_lock held */
 static struct jffs2_eraseblock *jffs2_find_gc_block(struct jffs2_sb_info 
*c)
@@ -169,6 +177,7 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info 
*c)
                if (!ic->pino_nlink) {
                        jffs2_dbg(1, "Skipping check of ino #%d with 
nlink/pino zero\n",
                                  ic->ino);
+                       ic->state = INO_STATE_DELETION;
                        spin_unlock(&c->inocache_lock);
                        jffs2_xattr_delete_inode(c, ic);
                        continue;
@@ -358,9 +367,14 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info 
*c)
 
        case INO_STATE_PRESENT:
                /* It's in-core. GC must iget() it. */
+       case INO_STATE_DELETION:
+               /* Unlinked node. GC must check deletion dirent. */
                break;
 
        case INO_STATE_UNCHECKED:
+               ic->state = INO_STATE_DELETION;
+               break;
+
        case INO_STATE_CHECKING:
        case INO_STATE_GC:
                /* Should never happen. We should have finished checking
@@ -431,19 +445,26 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info 
*c)
        nlink = ic->pino_nlink;
        spin_unlock(&c->inocache_lock);
 
-       f = jffs2_gc_fetch_inode(c, inum, !nlink);
+       if (ic->state == INO_STATE_DELETION)
+               f = NULL;
+       else
+               f = jffs2_gc_fetch_inode(c, inum, !nlink);
+
        if (IS_ERR(f)) {
                ret = PTR_ERR(f);
                goto release_sem;
        }
-       if (!f) {
+       if (!f && ic->state != INO_STATE_DELETION) {
                ret = 0;
                goto release_sem;
        }
 
-       ret = jffs2_garbage_collect_live(c, jeb, raw, f);
-
-       jffs2_gc_release_inode(c, f);
+       if (ic->state == INO_STATE_DELETION) {
+               ret = jffs2_garbage_collect_unlinked_dirent(c, jeb, raw, 
ic);
+       } else {
+               ret = jffs2_garbage_collect_live(c, jeb, raw, f);
+               jffs2_gc_release_inode(c, f);
+       }
 
  test_gcnode:
        if (jeb->dirty_size == gcblock_dirty && 
!ref_obsolete(jeb->gc_node)) {
@@ -990,6 +1011,236 @@ static int 
jffs2_garbage_collect_deletion_dirent(struct jffs2_sb_info *c, struct
        return 0;
 }
 
+static int jffs2_garbage_collect_unlinked_deletion(struct jffs2_sb_info 
*c,
+                                         struct jffs2_inode_cache *ic,
+                                         struct jffs2_raw_node_ref *raw,
+                                         struct jffs2_raw_dirent *rd)
+{
+       size_t retlen;
+       int ret;
+       uint32_t phys_ofs, alloclen;
+       uint32_t rawlen;
+       int retried = 0;
+
+       jffs2_dbg(1, "Going to GC REF_DELETION node at 0x%08x\n",
+                 ref_offset(raw));
+       alloclen = rawlen = ref_totlen(c, c->gcblock, raw);
+
+       /* Ask for a small amount of space (or the totlen if smaller) 
because we
+          don't want to force wastage of the end of a block if splitting 
would
+          work. */
+       if (ic && alloclen > sizeof(struct jffs2_raw_inode) + 
JFFS2_MIN_DATA_LEN)
+               alloclen = sizeof(struct jffs2_raw_inode) + 
JFFS2_MIN_DATA_LEN;
+
+       ret = jffs2_reserve_space_gc(c, alloclen, &alloclen, rawlen);
+
+       if (ret)
+               goto out_node;
+
+       if (alloclen < rawlen) {
+               ret = -EBADFD;
+               goto out_node;
+       }
+
+ retry:
+       phys_ofs = write_ofs(c);
+
+       ret = jffs2_flash_write(c, phys_ofs, rawlen, &retlen, (char *)rd);
+
+       if (ret || (retlen != rawlen)) {
+               pr_notice("Write of %d bytes at 0x%08x failed. returned 
%d, retlen %zd\n",
+                         rawlen, phys_ofs, ret, retlen);
+               if (retlen) {
+                       jffs2_add_physical_node_ref(c, phys_ofs | 
REF_OBSOLETE, rawlen, NULL);
+               } else {
+                       pr_notice("Not marking the space at 0x%08x as 
dirty because the flash driver returned retlen zero\n",
+                                 phys_ofs);
+               }
+               if (!retried) {
+                       /* Try to reallocate space and retry */
+                       uint32_t dummy;
+                       struct jffs2_eraseblock *jeb = &c->blocks[phys_ofs 
/ c->sector_size];
+
+                       retried = 1;
+
+                       jffs2_dbg(1, "Retrying failed write of 
REF_DELETION node.\n");
+
+                       jffs2_dbg_acct_sanity_check(c, jeb);
+                       jffs2_dbg_acct_paranoia_check(c, jeb);
+
+                       ret = jffs2_reserve_space_gc(c, rawlen, &dummy, 
rawlen);
+
+                       if (!ret) {
+                               jffs2_dbg(1, "Allocated space at 0x%08x to 
retry failed write.\n",
+                                         phys_ofs);
+
+                               jffs2_dbg_acct_sanity_check(c, jeb);
+                               jffs2_dbg_acct_paranoia_check(c, jeb);
+
+                               goto retry;
+                       }
+                       jffs2_dbg(1, "Failed to allocate space to retry 
failed write: %d!\n",
+                                 ret);
+               }
+
+               if (!ret)
+                       ret = -EIO;
+               goto out_node;
+       }
+       jffs2_add_physical_node_ref(c, phys_ofs | REF_NORMAL, rawlen, ic);
+
+       jffs2_mark_node_obsolete(c, raw);
+       jffs2_dbg(1, "WHEEE! GC REF_DELETION node at 0x%08x succeeded\n",
+                 ref_offset(raw));
+ out_node:
+       kfree(rd);
+       return ret;
+}
+
+static int jffs2_garbage_collect_unlinked_dirent(struct jffs2_sb_info *c,
+                               struct jffs2_eraseblock *jeb,
+                               struct jffs2_raw_node_ref *raw_suspect,
+                               struct jffs2_inode_cache *ic)
+{
+       struct jffs2_full_dirent *fd = NULL;
+       struct jffs2_raw_dirent *rd = NULL;
+       struct jffs2_raw_dirent *rd_suspect = NULL;
+
+       if (!jffs2_can_mark_obsolete(c)) {
+               int ret;
+               int name_len;
+               size_t retlen;
+               uint32_t checkedlen;
+               uint32_t name_crc;
+               uint32_t raw_len;
+               struct jffs2_raw_node_ref *raw;
+
+               raw_len = ref_totlen(c, jeb, raw_suspect);
+               rd_suspect = kmalloc(raw_len, GFP_KERNEL);
+               if (!rd_suspect)
+                       return -ENOMEM;
+
+               ret = jffs2_flash_read(c, ref_offset(raw_suspect),
+                       raw_len, &retlen, (char *)rd_suspect);
+
+               if (ret) {
+                       pr_warn("%s(): Read error (%d) reading deletion 
dirent node at %08x\n",
+                               __func__, ret, ref_offset(raw_suspect));
+                       kfree(rd_suspect);
+                       return -EIO;
+               }
+
+               if (je16_to_cpu(rd_suspect->nodetype) != 
JFFS2_NODETYPE_DIRENT) {
+                       pr_warn("%s(): ofs(%08x) nodetype(%04x) is not 
expected\n",
+                               __func__, ref_offset(raw_suspect),
+                               je16_to_cpu(rd_suspect->nodetype));
+                       kfree(rd_suspect);
+                       BUG();
+               }
+
+               if (je32_to_cpu(rd_suspect->ino)) {
+                       jffs2_dbg(1, "%s(): ino %d not deletion dirent\n",
+                               __func__, je32_to_cpu(rd_suspect->ino));
+                       goto obsolete;
+               }
+
+               checkedlen = strnlen(rd_suspect->name, rd_suspect->nsize);
+               fd = jffs2_alloc_full_dirent(checkedlen+1);
+               if (!fd) {
+                       kfree(rd_suspect);
+                       return -ENOMEM;
+               }
+
+               memcpy(&fd->name, rd_suspect->name, checkedlen);
+               fd->name[checkedlen] = 0;
+               fd->raw = raw_suspect;
+               name_len = strlen(fd->name);
+               name_crc = crc32(0, fd->name, name_len);
+               rd = kmalloc(raw_len, GFP_KERNEL);
+               if (!rd) {
+                       kfree(rd_suspect);
+                       jffs2_free_full_dirent(fd);
+                       return -ENOMEM;
+               }
+
+               /* Prevent the erase code from nicking the obsolete node 
refs
+               while we're looking at them. I really don't like this 
extra lock but
+               can't see any alternative. Suggestions on a postcard to... 
*/
+               mutex_lock(&c->erase_free_sem);
+               for (raw = ic->nodes; raw != (void *)ic; raw = 
raw->next_in_ino) {
+
+                       cond_resched();
+
+                       /* We only care about obsolete ones */
+                       if (!(ref_obsolete(raw)))
+                               continue;
+
+                       /* Any dirent with the same name is going to have 
the same length... */
+                       if (ref_totlen(c, NULL, raw) != raw_len)
+                               continue;
+
+                       /* Doesn't matter if there's one in the same erase 
block. We're going to
+                        delete it too at the same time. */
+                       if (SECTOR_ADDR(raw->flash_offset) == 
SECTOR_ADDR(fd->raw->flash_offset))
+                               continue;
+
+                       jffs2_dbg(1, "Check potential deletion dirent at 
%08x\n",
+                                 ref_offset(raw));
+
+                       /* This is an obsolete node belonging to the same 
directory, and it's of the right
+                          length. We need to take a closer look...*/
+                       ret = jffs2_flash_read(c, ref_offset(raw), 
raw_len, &retlen, (char *)rd);
+                       if (ret) {
+                               pr_warn("%s(): Read error (%d) reading 
obsolete node at %08x\n",
+                                       __func__, ret, ref_offset(raw));
+                               /* If we can't read it, we don't need to 
continue to obsolete it. Continue */
+                               continue;
+                       }
+                       if (retlen != raw_len) {
+                               pr_warn("%s(): Short read (%zd not %u) 
reading header from obsolete node at %08x\n",
+                                       __func__, retlen, raw_len,
+                                       ref_offset(raw));
+                               continue;
+                       }
+
+                       if (je16_to_cpu(rd->nodetype) != 
JFFS2_NODETYPE_DIRENT)
+                               continue;
+
+                       /* If the name CRC doesn't match, skip */
+                       if (je32_to_cpu(rd->name_crc) != name_crc)
+                               continue;
+
+                       /* If the name length doesn't match, or it's 
another deletion dirent, skip */
+                       if (rd->nsize != name_len || 
!je32_to_cpu(rd->ino))
+                               continue;
+
+                       /* OK, check the actual name now */
+                       if (memcmp(rd->name, fd->name, name_len))
+                               continue;
+
+                       /* OK. The name really does match. There really is 
still an older node on
+                          the flash which our deletion dirent obsoletes. 
So we have to write out
+                          a new deletion dirent to replace it */
+                       mutex_unlock(&c->erase_free_sem);
+
+                       jffs2_dbg(1, "Deletion dirent at %08x still 
obsoletes real dirent \"%s\" at %08x for ino #%u\n",
+                                 ref_offset(fd->raw), fd->name,
+                                 ref_offset(raw), je32_to_cpu(rd->ino));
+                       kfree(rd);
+                       jffs2_free_full_dirent(fd);
+                       return jffs2_garbage_collect_unlinked_deletion(c, 
ic, raw_suspect, rd_suspect);
+               }
+               mutex_unlock(&c->erase_free_sem);
+       }
+obsolete:
+       jffs2_mark_node_obsolete(c, raw_suspect);
+       if (fd)
+               jffs2_free_full_dirent(fd);
+       kfree(rd);
+       kfree(rd_suspect);
+       return 0;
+}
+
 static int jffs2_garbage_collect_hole(struct jffs2_sb_info *c, struct 
jffs2_eraseblock *jeb,
                                      struct jffs2_inode_info *f, struct 
jffs2_full_dnode *fn,
                                      uint32_t start, uint32_t end)
diff --git a/nodelist.h b/nodelist.h
index fa35ff7..e8e282a 100644
--- a/nodelist.h
+++ b/nodelist.h
@@ -192,6 +192,7 @@ struct jffs2_inode_cache {
 #define INO_STATE_GC           4       /* GCing a 'pristine' node */
 #define INO_STATE_READING      5       /* In read_inode() */
 #define INO_STATE_CLEARING     6       /* In clear_inode() */
+#define INO_STATE_DELETION     7       /* Unlinked inode */
 
 #define INO_FLAGS_XATTR_CHECKED        0x01    /* has no duplicate 
xattr_ref */
 
diff --git a/summary.c b/summary.c
index c522d09..2488d50 100644
--- a/summary.c
+++ b/summary.c
@@ -470,17 +470,85 @@ static int jffs2_sum_process_sum_data(struct 
jffs2_sb_info *c, struct jffs2_eras
                                        return -ENOMEM;
                                }
 
-                               fd->raw = sum_link_node_ref(c, jeb, 
je32_to_cpu(spd->offset) | REF_UNCHECKED,
- PAD(je32_to_cpu(spd->totlen)), ic);
-
+                               fd->ino = je32_to_cpu(spd->ino);
+                               if (fd->ino != 0) {
+                                       fd->raw = sum_link_node_ref(c, 
jeb,  je32_to_cpu(spd->offset) | REF_UNCHECKED,
+ PAD(je32_to_cpu(spd->totlen)), ic);
+                               } else {
+                                       int ret;
+                                       size_t retlen;
+                                       uint32_t hdr_crc, crc;
+                                       struct jffs2_raw_dirent *rd;
+                                       uint32_t rd_totlen = 
je32_to_cpu(spd->totlen);
+
+                                       rd = kmalloc(rd_totlen, 
GFP_KERNEL);
+                                       if (!rd) {
+ jffs2_free_full_dirent(fd);
+                                               return -ENOMEM;
+                                       }
+                                       ret = jffs2_flash_read(c, 
jeb->offset + je32_to_cpu(spd->offset),
+                                               rd_totlen, &retlen, (char 
*)rd);
+                                       if (!ret && retlen != rd_totlen) {
+                                               pr_err("Dirent at %08x has 
read problem. Aborting mount.\n",
+                                                       jeb->offset + 
je32_to_cpu(spd->offset));
+ jffs2_free_full_dirent(fd);
+                                               kfree(rd);
+                                               return -EIO;
+                                       }
+                                       hdr_crc = crc32(0, rd, 
sizeof(struct jffs2_unknown_node)-4);
+                                       if (hdr_crc != 
je32_to_cpu(rd->hdr_crc)) {
+                                               pr_notice("%s(): Node at 
0x%08x {0x%04x, 0x%04x, 0x%08x) has invalid CRC 0x%08x (calculated 
0x%08x)\n",
+                                                            __func__,
+                                                            jeb->offset + 
je32_to_cpu(spd->offset),
+ je16_to_cpu(rd->magic),
+ je16_to_cpu(rd->nodetype),
+ je32_to_cpu(rd->totlen),
+ je32_to_cpu(rd->hdr_crc),
+                                                            hdr_crc);
+ jffs2_free_full_dirent(fd);
+                                               kfree(rd);
+                                               err = 
jffs2_scan_dirty_space(c, jeb, PAD(rd_totlen));
+                                               if (err)
+                                                       return err;
+                                               goto out;
+                                       }
+                                       crc = crc32(0, rd, sizeof(*rd)-8);
+                                       if (crc != 
je32_to_cpu(rd->node_crc)) {
+                                               pr_notice("%s(): Node CRC 
failed on node at 0x%08x: Read 0x%08x, calculated 0x%08x\n",
+                                                         __func__, 
jeb->offset + je32_to_cpu(spd->offset),
+ je32_to_cpu(rd->node_crc), crc);
+ jffs2_free_full_dirent(fd);
+                                               kfree(rd);
+                                               err = 
jffs2_scan_dirty_space(c, jeb, PAD(rd_totlen));
+                                               if (err)
+                                                       return err;
+                                               goto out;
+                                       }
+                                       crc = crc32(0, fd->name, 
rd->nsize);
+                                       if (crc != 
je32_to_cpu(rd->name_crc)) {
+                                               pr_notice("%s(): Name CRC 
failed on node at 0x%08x: Read 0x%08x, calculated 0x%08x\n",
+                                                         __func__, 
jeb->offset + je32_to_cpu(spd->offset),
+ je32_to_cpu(rd->name_crc), crc);
+                                               jffs2_dbg(1, "Name for 
which CRC failed is (now) '%s', ino #%d\n",
+                                                         fd->name, 
je32_to_cpu(rd->ino));
+ jffs2_free_full_dirent(fd);
+                                               kfree(rd);
+                                               err = 
jffs2_scan_dirty_space(c, jeb, PAD(rd_totlen));
+                                               if (err)
+                                                       return err;
+                                               goto out;
+                                       }
+                                       kfree(rd);
+                                       fd->raw = sum_link_node_ref(c, 
jeb,  je32_to_cpu(spd->offset) | REF_NORMAL,
+ PAD(je32_to_cpu(spd->totlen)), ic);
+                               }
                                fd->next = NULL;
                                fd->version = je32_to_cpu(spd->version);
-                               fd->ino = je32_to_cpu(spd->ino);
                                fd->nhash = full_name_hash(fd->name, 
checkedlen);
                                fd->type = spd->type;
 
                                jffs2_add_fd_to_list(c, fd, 
&ic->scan_dents);
-
+out:
                                *pseudo_random += 
je32_to_cpu(spd->version);
 
                                sp += 
JFFS2_SUMMARY_DIRENT_SIZE(spd->nsize);
diff --git a/write.c b/write.c
index b634de4..750257a 100644
--- a/write.c
+++ b/write.c
@@ -648,7 +648,7 @@ int jffs2_do_unlink(struct jffs2_sb_info *c, struct 
jffs2_inode_info *dir_f,
                                                  fd->name,
                                                  dead_f->inocache->ino);
                                }
-                               if (fd->raw)
+                               if (fd->raw && fd->ino)
                                        jffs2_mark_node_obsolete(c, 
fd->raw);
                                jffs2_free_full_dirent(fd);
                        }
Signed-off-by: Liu Song <liu.song11 at zte.com.cn>  
--------------------------------------------------------
ZTE Information Security Notice: The information contained in this mail (and any attachment transmitted herewith) is privileged and confidential and is intended for the exclusive use of the addressee(s).  If you are not an intended recipient, any disclosure, reproduction, distribution or other dissemination or use of the information contained is strictly prohibited.  If you have received this mail in error, please delete it and notify us immediately.



More information about the linux-mtd mailing list