[PATCH 1/2] JFFS2: Process obsolete nodes as well as accurate ones

[PATCH 1/2] JFFS2: Process obsolete nodes as well as accurate ones

[petr.borsodi at i.cz]

From: Petr Borsodi <petr.borsodi@i.cz>

Obsolete nodes (ie. without the JFFS2_NODE_ACCURATE flag) were ignored because
they had seemingly invalid crc. This could lead to finding the phantom node
header in obsolete node data.

Signed-off-by: Petr Borsodi <petr.borsodi@i.cz>
---

 fs/jffs2/jffs2_1pass.c | 67 +++++++++++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 24 deletions(-)

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 5912cde838..819a44eeeb 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -1471,7 +1471,7 @@ static u32
 jffs2_1pass_build_lists(struct part_info * part)
 {
 	struct b_lists *pL;
-	struct jffs2_unknown_node *node;
+	union jffs2_node_union *node;
 	u32 nr_sectors;
 	u32 i;
 	u32 counter4 = 0;
@@ -1507,6 +1507,7 @@ jffs2_1pass_build_lists(struct part_info * part)
 #endif
 		/* Indicates a sector with a CLEANMARKER was found */
 		int clean_sector = 0;
+		struct jffs2_unknown_node crcnode;
 
 		/* Set buf_size to maximum length */
 		buf_size = DEFAULT_EMPTY_SCAN_SIZE;
@@ -1600,9 +1601,9 @@ jffs2_1pass_build_lists(struct part_info * part)
 			}
 			prevofs = ofs;
 			if (sector_ofs + part->sector_size <
-					ofs + sizeof(*node))
+					ofs + sizeof(struct jffs2_unknown_node))
 				break;
-			if (buf_ofs + buf_len < ofs + sizeof(*node)) {
+			if (buf_ofs + buf_len < ofs + sizeof(struct jffs2_unknown_node)) {
 				buf_len = min_t(uint32_t, buf_size, sector_ofs
 						+ part->sector_size - ofs);
 				get_fl_mem((u32)part->offset + ofs, buf_len,
@@ -1610,7 +1611,7 @@ jffs2_1pass_build_lists(struct part_info * part)
 				buf_ofs = ofs;
 			}
 
-			node = (struct jffs2_unknown_node *)&buf[ofs-buf_ofs];
+			node = (union jffs2_node_union *)&buf[ofs-buf_ofs];
 
 			if (*(uint32_t *)(&buf[ofs-buf_ofs]) == 0xffffffff) {
 				uint32_t inbuf_ofs;
@@ -1665,23 +1666,41 @@ jffs2_1pass_build_lists(struct part_info * part)
 			 * the 'clean_sector' flag.
 			 */
 			clean_sector = 0;
-			if (node->magic != JFFS2_MAGIC_BITMASK ||
-					!hdr_crc(node)) {
+			if (node->u.magic != JFFS2_MAGIC_BITMASK) {
+				ofs += 4;
+				counter4++;
+				continue;
+			}
+
+			crcnode.magic = node->u.magic;
+			crcnode.nodetype = node->u.nodetype | JFFS2_NODE_ACCURATE;
+			crcnode.totlen = node->u.totlen;
+			crcnode.hdr_crc = node->u.hdr_crc;
+			if (!hdr_crc(&crcnode)) {
 				ofs += 4;
 				counter4++;
 				continue;
 			}
-			if (ofs + node->totlen >
-					sector_ofs + part->sector_size) {
+
+			if (ofs + node->u.totlen > sector_ofs + part->sector_size) {
 				ofs += 4;
 				counter4++;
 				continue;
 			}
+
+			if (!(node->u.nodetype & JFFS2_NODE_ACCURATE)) {
+				DEBUGF("Obsolete node type: %x len %d offset 0x%x\n",
+					node->u.nodetype,
+					node->u.totlen, ofs);
+				ofs += ((node->u.totlen + 3) & ~3);
+				counterF++;
+				continue;
+			}
+
 			/* if its a fragment add it */
-			switch (node->nodetype) {
+			switch (node->u.nodetype) {
 			case JFFS2_NODETYPE_INODE:
-				if (buf_ofs + buf_len < ofs + sizeof(struct
-							jffs2_raw_inode)) {
+				if (buf_ofs + buf_len < ofs + sizeof(struct jffs2_raw_inode)) {
 					buf_len = min_t(uint32_t,
 							sizeof(struct jffs2_raw_inode),
 							sector_ofs +
@@ -1701,8 +1720,8 @@ jffs2_1pass_build_lists(struct part_info * part)
 					jffs2_free_cache(part);
 					return 0;
 				}
-				if (max_totlen < node->totlen)
-					max_totlen = node->totlen;
+				if (max_totlen < node->u.totlen)
+					max_totlen = node->u.totlen;
 				break;
 			case JFFS2_NODETYPE_DIRENT:
 				if (buf_ofs + buf_len < ofs + sizeof(struct
@@ -1711,7 +1730,7 @@ jffs2_1pass_build_lists(struct part_info * part)
 							 jffs2_raw_dirent *)
 							node)->nsize) {
 					buf_len = min_t(uint32_t,
-							node->totlen,
+							node->u.totlen,
 							sector_ofs +
 							part->sector_size -
 							ofs);
@@ -1736,17 +1755,17 @@ jffs2_1pass_build_lists(struct part_info * part)
 					jffs2_free_cache(part);
 					return 0;
 				}
-				if (max_totlen < node->totlen)
-					max_totlen = node->totlen;
+				if (max_totlen < node->u.totlen)
+					max_totlen = node->u.totlen;
 				counterN++;
 				break;
 			case JFFS2_NODETYPE_CLEANMARKER:
-				if (node->totlen != sizeof(struct jffs2_unknown_node))
+				if (node->u.totlen != sizeof(struct jffs2_unknown_node))
 					printf("OOPS Cleanmarker has bad size "
 						"%d != %zu\n",
-						node->totlen,
+						node->u.totlen,
 						sizeof(struct jffs2_unknown_node));
-				if ((node->totlen ==
+				if ((node->u.totlen ==
 				     sizeof(struct jffs2_unknown_node)) &&
 				    (ofs == sector_ofs)) {
 					/*
@@ -1758,20 +1777,20 @@ jffs2_1pass_build_lists(struct part_info * part)
 				}
 				break;
 			case JFFS2_NODETYPE_PADDING:
-				if (node->totlen < sizeof(struct jffs2_unknown_node))
+				if (node->u.totlen < sizeof(struct jffs2_unknown_node))
 					printf("OOPS Padding has bad size "
 						"%d < %zu\n",
-						node->totlen,
+						node->u.totlen,
 						sizeof(struct jffs2_unknown_node));
 				break;
 			case JFFS2_NODETYPE_SUMMARY:
 				break;
 			default:
 				printf("Unknown node type: %x len %d offset 0x%x\n",
-					node->nodetype,
-					node->totlen, ofs);
+					node->u.nodetype,
+					node->u.totlen, ofs);
 			}
-			ofs += ((node->totlen + 3) & ~3);
+			ofs += ((node->u.totlen + 3) & ~3);
 			counterF++;
 		}
 	}
-- 
2.20.1

[PATCH 2/2] JFFS2: Add useful fields into lists

[petr.borsodi at i.cz]

From: Petr Borsodi <petr.borsodi@i.cz>

The inode list uses version and ino, the dirent list uses version and pino.
This information is collected during scanning, reducing accesses to flash
and significantly speeding up ls and read.

Signed-off-by: Petr Borsodi <petr.borsodi@i.cz>
---

 fs/jffs2/jffs2_1pass.c   | 154 +++++++++++++++++++--------------------
 fs/jffs2/jffs2_private.h |   5 ++
 2 files changed, 81 insertions(+), 78 deletions(-)

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 819a44eeeb..3b98384b8b 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -549,7 +549,7 @@ add_node(struct b_list *list)
 }
 
 static struct b_node *
-insert_node(struct b_list *list, u32 offset)
+insert_node(struct b_list *list)
 {
 	struct b_node *new;
 
@@ -557,7 +557,6 @@ insert_node(struct b_list *list, u32 offset)
 		putstr("add_node failed!\r\n");
 		return NULL;
 	}
-	new->offset = offset;
 	new->next = NULL;
 
 	if (list->listTail != NULL)
@@ -575,18 +574,7 @@ insert_node(struct b_list *list, u32 offset)
  */
 static int compare_inodes(struct b_node *new, struct b_node *old)
 {
-	/*
-	 * Only read in the version info from flash, not the entire inode.
-	 * This can make a big difference to speed if flash is slow.
-	 */
-	u32 new_version;
-	u32 old_version;
-	get_fl_mem(new->offset + offsetof(struct jffs2_raw_inode, version),
-		   sizeof(new_version), &new_version);
-	get_fl_mem(old->offset + offsetof(struct jffs2_raw_inode, version),
-		   sizeof(old_version), &old_version);
-
-	return new_version > old_version;
+	return new->version > old->version;
 }
 
 /* Sort directory entries so all entries in the same directory
@@ -683,7 +671,7 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 	uchar *src;
 	int i;
 	u32 counter = 0;
-#ifdef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
+
 	/* Find file size before loading any data, so fragments that
 	 * start past the end of file can be ignored. A fragment
 	 * that is partially in the file is loaded, so extra data may
@@ -691,35 +679,40 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 	 * This shouldn't cause trouble when loading kernel images, so
 	 * we will live with it.
 	 */
+	int latestOffset = -1;
 	for (b = pL->frag.listHead; b != NULL; b = b->next) {
-		jNode = (struct jffs2_raw_inode *) get_fl_mem(b->offset,
-			sizeof(struct jffs2_raw_inode), pL->readbuf);
-		if ((inode == jNode->ino)) {
+		if ((inode == b->ino)) {
 			/* get actual file length from the newest node */
-			if (jNode->version >= latestVersion) {
-				totalSize = jNode->isize;
-				latestVersion = jNode->version;
+			if (b->version >= latestVersion) {
+				latestVersion = b->version;
+				latestOffset = b->offset;
 			}
 		}
+	}
+
+	if (latestOffset >= 0) {
+		jNode = (struct jffs2_raw_inode *) get_fl_mem(latestOffset,
+			sizeof(struct jffs2_raw_inode), pL->readbuf);
+		totalSize = jNode->isize;
 		put_fl_mem(jNode, pL->readbuf);
 	}
+
 	/*
 	 * If no destination is provided, we are done.
 	 * Just return the total size.
 	 */
 	if (!dest)
 		return totalSize;
-#endif
 
 	for (b = pL->frag.listHead; b != NULL; b = b->next) {
-		/*
-		 * Copy just the node and not the data at this point,
-		 * since we don't yet know if we need this data.
-		 */
-		jNode = (struct jffs2_raw_inode *)get_fl_mem(b->offset,
-				sizeof(struct jffs2_raw_inode),
-				pL->readbuf);
-		if (inode == jNode->ino) {
+		if (inode == b->ino) {
+			/*
+			 * Copy just the node and not the data at this point,
+			 * since we don't yet know if we need this data.
+			 */
+			jNode = (struct jffs2_raw_inode *)get_fl_mem(b->offset,
+					sizeof(struct jffs2_raw_inode),
+					pL->readbuf);
 #if 0
 			putLabeledWord("\r\n\r\nread_inode: totlen = ", jNode->totlen);
 			putLabeledWord("read_inode: inode = ", jNode->ino);
@@ -733,14 +726,6 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 			putLabeledWord("read_inode: flags = ", jNode->flags);
 #endif
 
-#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
-			/* get actual file length from the newest node */
-			if (jNode->version >= latestVersion) {
-				totalSize = jNode->isize;
-				latestVersion = jNode->version;
-			}
-#endif
-
 			if(dest) {
 				/*
 				 * Now that the inode has been checked,
@@ -804,9 +789,9 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 #if 0
 			putLabeledWord("read_inode: totalSize = ", totalSize);
 #endif
+			put_fl_mem(jNode, pL->readbuf);
 		}
 		counter++;
-		put_fl_mem(jNode, pL->readbuf);
 	}
 
 #if 0
@@ -953,13 +938,14 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino)
 	struct jffs2_raw_dirent *jDir;
 
 	for (b = pL->dir.listHead; b; b = b->next) {
-		jDir = (struct jffs2_raw_dirent *) get_node_mem(b->offset,
-								pL->readbuf);
-		if (pino == jDir->pino) {
+		if (pino == b->pino) {
 			u32 i_version = 0;
-			struct jffs2_raw_inode *jNode, *i = NULL;
+			int i_offset = -1;
+			struct jffs2_raw_inode *jNode = NULL;
 			struct b_node *b2;
 
+			jDir = (struct jffs2_raw_dirent *)
+				get_node_mem(b->offset, pL->readbuf);
 #ifdef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
 			/* Check for more recent versions of this file */
 			int match;
@@ -991,30 +977,27 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino)
 			}
 
 			for (b2 = pL->frag.listHead; b2; b2 = b2->next) {
-				jNode = (struct jffs2_raw_inode *)
-					get_fl_mem(b2->offset, sizeof(*jNode),
-						   NULL);
-				if (jNode->ino == jDir->ino &&
-				    jNode->version >= i_version) {
-					i_version = jNode->version;
-					if (i)
-						put_fl_mem(i, NULL);
-
-					if (jDir->type == DT_LNK)
-						i = get_node_mem(b2->offset,
-								 NULL);
-					else
-						i = get_fl_mem(b2->offset,
-							       sizeof(*i),
-							       NULL);
+				if (b2->ino == jDir->ino &&
+				    b2->version >= i_version) {
+					i_version = b2->version;
+					i_offset = b2->offset;
 				}
-				put_fl_mem(jNode, NULL);
 			}
 
-			dump_inode(pL, jDir, i);
-			put_fl_mem(i, NULL);
+			if (i_version >= 0) {
+				if (jDir->type == DT_LNK)
+					jNode = get_node_mem(i_offset, NULL);
+				else
+					jNode = get_fl_mem(i_offset,
+							   sizeof(*jNode),
+							   NULL);
+			}
+
+			dump_inode(pL, jDir, jNode);
+			put_fl_mem(jNode, NULL);
+
+			put_fl_mem(jDir, pL->readbuf);
 		}
-		put_fl_mem(jDir, pL->readbuf);
 	}
 	return pino;
 }
@@ -1266,7 +1249,7 @@ static int jffs2_sum_process_sum_data(struct part_info *part, uint32_t offset,
 {
 	void *sp;
 	int i, pass;
-	void *ret;
+	struct b_node *b;
 
 	for (pass = 0; pass < 2; pass++) {
 		sp = summary->sum;
@@ -1281,13 +1264,17 @@ static int jffs2_sum_process_sum_data(struct part_info *part, uint32_t offset,
 					if (pass) {
 						spi = sp;
 
-						ret = insert_node(&pL->frag,
-							(u32)part->offset +
+						b = insert_node(&pL->frag);
+						if (b == NULL)
+							return -1;
+						b->offset = (u32)part->offset +
 							offset +
 							sum_get_unaligned32(
-								&spi->offset));
-						if (ret == NULL)
-							return -1;
+								&spi->offset);
+						b->version = sum_get_unaligned32(
+							&spi->version);
+						b->ino = sum_get_unaligned32(
+							&spi->inode);
 					}
 
 					sp += JFFS2_SUMMARY_INODE_SIZE;
@@ -1298,13 +1285,17 @@ static int jffs2_sum_process_sum_data(struct part_info *part, uint32_t offset,
 					struct jffs2_sum_dirent_flash *spd;
 					spd = sp;
 					if (pass) {
-						ret = insert_node(&pL->dir,
-							(u32) part->offset +
+						b = insert_node(&pL->dir);
+						if (b == NULL)
+							return -1;
+						b->offset = (u32)part->offset +
 							offset +
 							sum_get_unaligned32(
-								&spd->offset));
-						if (ret == NULL)
-							return -1;
+								&spd->offset);
+						b->version = sum_get_unaligned32(
+							&spd->version);
+						b->pino = sum_get_unaligned32(
+							&spd->pino);
 					}
 
 					sp += JFFS2_SUMMARY_DIRENT_SIZE(
@@ -1508,6 +1499,7 @@ jffs2_1pass_build_lists(struct part_info * part)
 		/* Indicates a sector with a CLEANMARKER was found */
 		int clean_sector = 0;
 		struct jffs2_unknown_node crcnode;
+		struct b_node *b;
 
 		/* Set buf_size to maximum length */
 		buf_size = DEFAULT_EMPTY_SCAN_SIZE;
@@ -1714,12 +1706,15 @@ jffs2_1pass_build_lists(struct part_info * part)
 				if (!inode_crc((struct jffs2_raw_inode *)node))
 					break;
 
-				if (insert_node(&pL->frag, (u32) part->offset +
-						ofs) == NULL) {
+				b = insert_node(&pL->frag);
+				if (!b) {
 					free(buf);
 					jffs2_free_cache(part);
 					return 0;
 				}
+				b->offset = (u32) part->offset + ofs;
+				b->version = node->i.version;
+				b->ino = node->i.ino;
 				if (max_totlen < node->u.totlen)
 					max_totlen = node->u.totlen;
 				break;
@@ -1749,12 +1744,15 @@ jffs2_1pass_build_lists(struct part_info * part)
 					break;
 				if (! (counterN%100))
 					puts ("\b\b.  ");
-				if (insert_node(&pL->dir, (u32) part->offset +
-						ofs) == NULL) {
+				b = insert_node(&pL->dir);
+				if (!b) {
 					free(buf);
 					jffs2_free_cache(part);
 					return 0;
 				}
+				b->offset = (u32) part->offset + ofs;
+				b->version = node->d.version;
+				b->pino = node->d.pino;
 				if (max_totlen < node->u.totlen)
 					max_totlen = node->u.totlen;
 				counterN++;
diff --git a/fs/jffs2/jffs2_private.h b/fs/jffs2/jffs2_private.h
index 06b6ca2919..65d19a76f9 100644
--- a/fs/jffs2/jffs2_private.h
+++ b/fs/jffs2/jffs2_private.h
@@ -8,6 +8,11 @@ struct b_node {
 	u32 offset;
 	struct b_node *next;
 	enum { CRC_UNKNOWN = 0, CRC_OK, CRC_BAD } datacrc;
+	u32 version;
+	union {
+		u32 ino; /* for inodes */
+		u32 pino; /* for dirents */
+	};
 };
 
 struct b_list {
-- 
2.20.1

[PATCH 1/2] JFFS2: Process obsolete nodes as well as accurate ones

[Tom Rini]

On Mon, Apr 27, 2020 at 06:43:25PM +0200, petr.borsodi at i.cz wrote:

> From: Petr Borsodi <petr.borsodi@i.cz>
> 
> Obsolete nodes (ie. without the JFFS2_NODE_ACCURATE flag) were ignored because
> they had seemingly invalid crc. This could lead to finding the phantom node
> header in obsolete node data.
> 
> Signed-off-by: Petr Borsodi <petr.borsodi@i.cz>

This (and 2/2) have a number of checkpatch warnings and while some of
them cannot be fixed easily (such as CamelCase stuff that you're
re-using and not introducing) others such as spacing, NULL comparisons,
etc, need to be fixed.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200506/c1bca302/attachment.sig>

[PATCH v2 1/2] JFFS2: Process obsolete nodes as well as accurate ones

[Petr Borsodi]

Obsolete nodes (ie. without the JFFS2_NODE_ACCURATE flag) were ignored
because they had seemingly invalid crc. This could lead to finding
the phantom node header in obsolete node data.

Signed-off-by: Petr Borsodi <petr.borsodi@i.cz>
---

Changes for v2:
   - Coding Style cleanup

 fs/jffs2/jffs2_1pass.c | 73 +++++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 26 deletions(-)

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 5912cde838..68db8c3ba4 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -1471,7 +1471,7 @@ static u32
 jffs2_1pass_build_lists(struct part_info * part)
 {
 	struct b_lists *pL;
-	struct jffs2_unknown_node *node;
+	union jffs2_node_union *node;
 	u32 nr_sectors;
 	u32 i;
 	u32 counter4 = 0;
@@ -1507,6 +1507,7 @@ jffs2_1pass_build_lists(struct part_info * part)
 #endif
 		/* Indicates a sector with a CLEANMARKER was found */
 		int clean_sector = 0;
+		struct jffs2_unknown_node crcnode;
 
 		/* Set buf_size to maximum length */
 		buf_size = DEFAULT_EMPTY_SCAN_SIZE;
@@ -1600,9 +1601,10 @@ jffs2_1pass_build_lists(struct part_info * part)
 			}
 			prevofs = ofs;
 			if (sector_ofs + part->sector_size <
-					ofs + sizeof(*node))
+					ofs + sizeof(struct jffs2_unknown_node))
 				break;
-			if (buf_ofs + buf_len < ofs + sizeof(*node)) {
+			if (buf_ofs + buf_len <
+					ofs + sizeof(struct jffs2_unknown_node)) {
 				buf_len = min_t(uint32_t, buf_size, sector_ofs
 						+ part->sector_size - ofs);
 				get_fl_mem((u32)part->offset + ofs, buf_len,
@@ -1610,7 +1612,7 @@ jffs2_1pass_build_lists(struct part_info * part)
 				buf_ofs = ofs;
 			}
 
-			node = (struct jffs2_unknown_node *)&buf[ofs-buf_ofs];
+			node = (union jffs2_node_union *)&buf[ofs - buf_ofs];
 
 			if (*(uint32_t *)(&buf[ofs-buf_ofs]) == 0xffffffff) {
 				uint32_t inbuf_ofs;
@@ -1665,23 +1667,41 @@ jffs2_1pass_build_lists(struct part_info * part)
 			 * the 'clean_sector' flag.
 			 */
 			clean_sector = 0;
-			if (node->magic != JFFS2_MAGIC_BITMASK ||
-					!hdr_crc(node)) {
+			if (node->u.magic != JFFS2_MAGIC_BITMASK) {
+				ofs += 4;
+				counter4++;
+				continue;
+			}
+
+			crcnode.magic = node->u.magic;
+			crcnode.nodetype = node->u.nodetype | JFFS2_NODE_ACCURATE;
+			crcnode.totlen = node->u.totlen;
+			crcnode.hdr_crc = node->u.hdr_crc;
+			if (!hdr_crc(&crcnode)) {
 				ofs += 4;
 				counter4++;
 				continue;
 			}
-			if (ofs + node->totlen >
-					sector_ofs + part->sector_size) {
+
+			if (ofs + node->u.totlen > sector_ofs + part->sector_size) {
 				ofs += 4;
 				counter4++;
 				continue;
 			}
+
+			if (!(node->u.nodetype & JFFS2_NODE_ACCURATE)) {
+				DEBUGF("Obsolete node type: %x len %d offset 0x%x\n",
+				       node->u.nodetype, node->u.totlen, ofs);
+				ofs += ((node->u.totlen + 3) & ~3);
+				counterF++;
+				continue;
+			}
+
 			/* if its a fragment add it */
-			switch (node->nodetype) {
+			switch (node->u.nodetype) {
 			case JFFS2_NODETYPE_INODE:
-				if (buf_ofs + buf_len < ofs + sizeof(struct
-							jffs2_raw_inode)) {
+				if (buf_ofs + buf_len <
+					ofs + sizeof(struct jffs2_raw_inode)) {
 					buf_len = min_t(uint32_t,
 							sizeof(struct jffs2_raw_inode),
 							sector_ofs +
@@ -1701,8 +1721,8 @@ jffs2_1pass_build_lists(struct part_info * part)
 					jffs2_free_cache(part);
 					return 0;
 				}
-				if (max_totlen < node->totlen)
-					max_totlen = node->totlen;
+				if (max_totlen < node->u.totlen)
+					max_totlen = node->u.totlen;
 				break;
 			case JFFS2_NODETYPE_DIRENT:
 				if (buf_ofs + buf_len < ofs + sizeof(struct
@@ -1711,7 +1731,7 @@ jffs2_1pass_build_lists(struct part_info * part)
 							 jffs2_raw_dirent *)
 							node)->nsize) {
 					buf_len = min_t(uint32_t,
-							node->totlen,
+							node->u.totlen,
 							sector_ofs +
 							part->sector_size -
 							ofs);
@@ -1736,19 +1756,19 @@ jffs2_1pass_build_lists(struct part_info * part)
 					jffs2_free_cache(part);
 					return 0;
 				}
-				if (max_totlen < node->totlen)
-					max_totlen = node->totlen;
+				if (max_totlen < node->u.totlen)
+					max_totlen = node->u.totlen;
 				counterN++;
 				break;
 			case JFFS2_NODETYPE_CLEANMARKER:
-				if (node->totlen != sizeof(struct jffs2_unknown_node))
+				if (node->u.totlen != sizeof(struct jffs2_unknown_node))
 					printf("OOPS Cleanmarker has bad size "
 						"%d != %zu\n",
-						node->totlen,
+						node->u.totlen,
 						sizeof(struct jffs2_unknown_node));
-				if ((node->totlen ==
-				     sizeof(struct jffs2_unknown_node)) &&
-				    (ofs == sector_ofs)) {
+				if (node->u.totlen ==
+				     sizeof(struct jffs2_unknown_node) &&
+				    ofs == sector_ofs) {
 					/*
 					 * Found a CLEANMARKER at the beginning
 					 * of the sector. It's in the correct
@@ -1758,20 +1778,21 @@ jffs2_1pass_build_lists(struct part_info * part)
 				}
 				break;
 			case JFFS2_NODETYPE_PADDING:
-				if (node->totlen < sizeof(struct jffs2_unknown_node))
+				if (node->u.totlen <
+						sizeof(struct jffs2_unknown_node))
 					printf("OOPS Padding has bad size "
 						"%d < %zu\n",
-						node->totlen,
+						node->u.totlen,
 						sizeof(struct jffs2_unknown_node));
 				break;
 			case JFFS2_NODETYPE_SUMMARY:
 				break;
 			default:
 				printf("Unknown node type: %x len %d offset 0x%x\n",
-					node->nodetype,
-					node->totlen, ofs);
+					node->u.nodetype,
+					node->u.totlen, ofs);
 			}
-			ofs += ((node->totlen + 3) & ~3);
+			ofs += ((node->u.totlen + 3) & ~3);
 			counterF++;
 		}
 	}
-- 
2.20.1

[PATCH v2 2/2] JFFS2: Add useful fields into lists

[Petr Borsodi]

The inode list uses version and ino, the dirent list uses version and pino.
This information is collected during scanning, reducing accesses to flash
and significantly speeding up ls and read.

Signed-off-by: Petr Borsodi <petr.borsodi@i.cz>
---

Changes for v2:
   - Coding Style cleanup

 fs/jffs2/jffs2_1pass.c   | 154 +++++++++++++++++++--------------------
 fs/jffs2/jffs2_private.h |   5 ++
 2 files changed, 81 insertions(+), 78 deletions(-)

diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
index 68db8c3ba4..1115c8682e 100644
--- a/fs/jffs2/jffs2_1pass.c
+++ b/fs/jffs2/jffs2_1pass.c
@@ -549,7 +549,7 @@ add_node(struct b_list *list)
 }
 
 static struct b_node *
-insert_node(struct b_list *list, u32 offset)
+insert_node(struct b_list *list)
 {
 	struct b_node *new;
 
@@ -557,7 +557,6 @@ insert_node(struct b_list *list, u32 offset)
 		putstr("add_node failed!\r\n");
 		return NULL;
 	}
-	new->offset = offset;
 	new->next = NULL;
 
 	if (list->listTail != NULL)
@@ -575,18 +574,7 @@ insert_node(struct b_list *list, u32 offset)
  */
 static int compare_inodes(struct b_node *new, struct b_node *old)
 {
-	/*
-	 * Only read in the version info from flash, not the entire inode.
-	 * This can make a big difference to speed if flash is slow.
-	 */
-	u32 new_version;
-	u32 old_version;
-	get_fl_mem(new->offset + offsetof(struct jffs2_raw_inode, version),
-		   sizeof(new_version), &new_version);
-	get_fl_mem(old->offset + offsetof(struct jffs2_raw_inode, version),
-		   sizeof(old_version), &old_version);
-
-	return new_version > old_version;
+	return new->version > old->version;
 }
 
 /* Sort directory entries so all entries in the same directory
@@ -683,7 +671,7 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 	uchar *src;
 	int i;
 	u32 counter = 0;
-#ifdef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
+
 	/* Find file size before loading any data, so fragments that
 	 * start past the end of file can be ignored. A fragment
 	 * that is partially in the file is loaded, so extra data may
@@ -691,35 +679,40 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 	 * This shouldn't cause trouble when loading kernel images, so
 	 * we will live with it.
 	 */
+	int latestOffset = -1;
 	for (b = pL->frag.listHead; b != NULL; b = b->next) {
-		jNode = (struct jffs2_raw_inode *) get_fl_mem(b->offset,
-			sizeof(struct jffs2_raw_inode), pL->readbuf);
-		if ((inode == jNode->ino)) {
+		if (inode == b->ino) {
 			/* get actual file length from the newest node */
-			if (jNode->version >= latestVersion) {
-				totalSize = jNode->isize;
-				latestVersion = jNode->version;
+			if (b->version >= latestVersion) {
+				latestVersion = b->version;
+				latestOffset = b->offset;
 			}
 		}
+	}
+
+	if (latestOffset >= 0) {
+		jNode = (struct jffs2_raw_inode *)get_fl_mem(latestOffset,
+			sizeof(struct jffs2_raw_inode), pL->readbuf);
+		totalSize = jNode->isize;
 		put_fl_mem(jNode, pL->readbuf);
 	}
+
 	/*
 	 * If no destination is provided, we are done.
 	 * Just return the total size.
 	 */
 	if (!dest)
 		return totalSize;
-#endif
 
 	for (b = pL->frag.listHead; b != NULL; b = b->next) {
-		/*
-		 * Copy just the node and not the data at this point,
-		 * since we don't yet know if we need this data.
-		 */
-		jNode = (struct jffs2_raw_inode *)get_fl_mem(b->offset,
-				sizeof(struct jffs2_raw_inode),
-				pL->readbuf);
-		if (inode == jNode->ino) {
+		if (inode == b->ino) {
+			/*
+			 * Copy just the node and not the data at this point,
+			 * since we don't yet know if we need this data.
+			 */
+			jNode = (struct jffs2_raw_inode *)get_fl_mem(b->offset,
+					sizeof(struct jffs2_raw_inode),
+					pL->readbuf);
 #if 0
 			putLabeledWord("\r\n\r\nread_inode: totlen = ", jNode->totlen);
 			putLabeledWord("read_inode: inode = ", jNode->ino);
@@ -733,14 +726,6 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 			putLabeledWord("read_inode: flags = ", jNode->flags);
 #endif
 
-#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
-			/* get actual file length from the newest node */
-			if (jNode->version >= latestVersion) {
-				totalSize = jNode->isize;
-				latestVersion = jNode->version;
-			}
-#endif
-
 			if(dest) {
 				/*
 				 * Now that the inode has been checked,
@@ -804,9 +789,9 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
 #if 0
 			putLabeledWord("read_inode: totalSize = ", totalSize);
 #endif
+			put_fl_mem(jNode, pL->readbuf);
 		}
 		counter++;
-		put_fl_mem(jNode, pL->readbuf);
 	}
 
 #if 0
@@ -953,13 +938,14 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino)
 	struct jffs2_raw_dirent *jDir;
 
 	for (b = pL->dir.listHead; b; b = b->next) {
-		jDir = (struct jffs2_raw_dirent *) get_node_mem(b->offset,
-								pL->readbuf);
-		if (pino == jDir->pino) {
+		if (pino == b->pino) {
 			u32 i_version = 0;
-			struct jffs2_raw_inode *jNode, *i = NULL;
+			int i_offset = -1;
+			struct jffs2_raw_inode *jNode = NULL;
 			struct b_node *b2;
 
+			jDir = (struct jffs2_raw_dirent *)
+				get_node_mem(b->offset, pL->readbuf);
 #ifdef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
 			/* Check for more recent versions of this file */
 			int match;
@@ -991,30 +977,27 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino)
 			}
 
 			for (b2 = pL->frag.listHead; b2; b2 = b2->next) {
-				jNode = (struct jffs2_raw_inode *)
-					get_fl_mem(b2->offset, sizeof(*jNode),
-						   NULL);
-				if (jNode->ino == jDir->ino &&
-				    jNode->version >= i_version) {
-					i_version = jNode->version;
-					if (i)
-						put_fl_mem(i, NULL);
-
-					if (jDir->type == DT_LNK)
-						i = get_node_mem(b2->offset,
-								 NULL);
-					else
-						i = get_fl_mem(b2->offset,
-							       sizeof(*i),
-							       NULL);
+				if (b2->ino == jDir->ino &&
+				    b2->version >= i_version) {
+					i_version = b2->version;
+					i_offset = b2->offset;
 				}
-				put_fl_mem(jNode, NULL);
 			}
 
-			dump_inode(pL, jDir, i);
-			put_fl_mem(i, NULL);
+			if (i_version >= 0) {
+				if (jDir->type == DT_LNK)
+					jNode = get_node_mem(i_offset, NULL);
+				else
+					jNode = get_fl_mem(i_offset,
+							   sizeof(*jNode),
+							   NULL);
+			}
+
+			dump_inode(pL, jDir, jNode);
+			put_fl_mem(jNode, NULL);
+
+			put_fl_mem(jDir, pL->readbuf);
 		}
-		put_fl_mem(jDir, pL->readbuf);
 	}
 	return pino;
 }
@@ -1266,7 +1249,7 @@ static int jffs2_sum_process_sum_data(struct part_info *part, uint32_t offset,
 {
 	void *sp;
 	int i, pass;
-	void *ret;
+	struct b_node *b;
 
 	for (pass = 0; pass < 2; pass++) {
 		sp = summary->sum;
@@ -1281,13 +1264,17 @@ static int jffs2_sum_process_sum_data(struct part_info *part, uint32_t offset,
 					if (pass) {
 						spi = sp;
 
-						ret = insert_node(&pL->frag,
-							(u32)part->offset +
+						b = insert_node(&pL->frag);
+						if (!b)
+							return -1;
+						b->offset = (u32)part->offset +
 							offset +
 							sum_get_unaligned32(
-								&spi->offset));
-						if (ret == NULL)
-							return -1;
+								&spi->offset);
+						b->version = sum_get_unaligned32(
+							&spi->version);
+						b->ino = sum_get_unaligned32(
+							&spi->inode);
 					}
 
 					sp += JFFS2_SUMMARY_INODE_SIZE;
@@ -1298,13 +1285,17 @@ static int jffs2_sum_process_sum_data(struct part_info *part, uint32_t offset,
 					struct jffs2_sum_dirent_flash *spd;
 					spd = sp;
 					if (pass) {
-						ret = insert_node(&pL->dir,
-							(u32) part->offset +
+						b = insert_node(&pL->dir);
+						if (!b)
+							return -1;
+						b->offset = (u32)part->offset +
 							offset +
 							sum_get_unaligned32(
-								&spd->offset));
-						if (ret == NULL)
-							return -1;
+								&spd->offset);
+						b->version = sum_get_unaligned32(
+							&spd->version);
+						b->pino = sum_get_unaligned32(
+							&spd->pino);
 					}
 
 					sp += JFFS2_SUMMARY_DIRENT_SIZE(
@@ -1508,6 +1499,7 @@ jffs2_1pass_build_lists(struct part_info * part)
 		/* Indicates a sector with a CLEANMARKER was found */
 		int clean_sector = 0;
 		struct jffs2_unknown_node crcnode;
+		struct b_node *b;
 
 		/* Set buf_size to maximum length */
 		buf_size = DEFAULT_EMPTY_SCAN_SIZE;
@@ -1715,12 +1707,15 @@ jffs2_1pass_build_lists(struct part_info * part)
 				if (!inode_crc((struct jffs2_raw_inode *)node))
 					break;
 
-				if (insert_node(&pL->frag, (u32) part->offset +
-						ofs) == NULL) {
+				b = insert_node(&pL->frag);
+				if (!b) {
 					free(buf);
 					jffs2_free_cache(part);
 					return 0;
 				}
+				b->offset = (u32)part->offset + ofs;
+				b->version = node->i.version;
+				b->ino = node->i.ino;
 				if (max_totlen < node->u.totlen)
 					max_totlen = node->u.totlen;
 				break;
@@ -1750,12 +1745,15 @@ jffs2_1pass_build_lists(struct part_info * part)
 					break;
 				if (! (counterN%100))
 					puts ("\b\b.  ");
-				if (insert_node(&pL->dir, (u32) part->offset +
-						ofs) == NULL) {
+				b = insert_node(&pL->dir);
+				if (!b) {
 					free(buf);
 					jffs2_free_cache(part);
 					return 0;
 				}
+				b->offset = (u32)part->offset + ofs;
+				b->version = node->d.version;
+				b->pino = node->d.pino;
 				if (max_totlen < node->u.totlen)
 					max_totlen = node->u.totlen;
 				counterN++;
diff --git a/fs/jffs2/jffs2_private.h b/fs/jffs2/jffs2_private.h
index 06b6ca2919..65d19a76f9 100644
--- a/fs/jffs2/jffs2_private.h
+++ b/fs/jffs2/jffs2_private.h
@@ -8,6 +8,11 @@ struct b_node {
 	u32 offset;
 	struct b_node *next;
 	enum { CRC_UNKNOWN = 0, CRC_OK, CRC_BAD } datacrc;
+	u32 version;
+	union {
+		u32 ino; /* for inodes */
+		u32 pino; /* for dirents */
+	};
 };
 
 struct b_list {
-- 
2.20.1

[PATCH v2 1/2] JFFS2: Process obsolete nodes as well as accurate ones

[Tom Rini]

On Thu, May 07, 2020 at 12:25:55PM +0200, Petr Borsodi wrote:

> Obsolete nodes (ie. without the JFFS2_NODE_ACCURATE flag) were ignored
> because they had seemingly invalid crc. This could lead to finding
> the phantom node header in obsolete node data.
> 
> Signed-off-by: Petr Borsodi <petr.borsodi@i.cz>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200515/3c66a5d0/attachment.sig>

[PATCH v2 2/2] JFFS2: Add useful fields into lists

[Tom Rini]

On Thu, May 07, 2020 at 12:25:56PM +0200, Petr Borsodi wrote:

> The inode list uses version and ino, the dirent list uses version and pino.
> This information is collected during scanning, reducing accesses to flash
> and significantly speeding up ls and read.
> 
> Signed-off-by: Petr Borsodi <petr.borsodi@i.cz>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200515/7bb74cd7/attachment.sig>