Patchwork [5/7] UBIFS: do not look up junk nodes

login
register
mail settings
Submitter Artem Bityutskiy
Date Aug. 7, 2010, 8:26 a.m.
Message ID <1281169577-18664-6-git-send-email-dedekind1@gmail.com>
Download mbox | patch
Permalink /patch/61166/
State New
Headers show

Comments

Artem Bityutskiy - Aug. 7, 2010, 8:26 a.m.
From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

When moving nodes, do not try to look up truncation and padding nodes in TNC,
because they do not exist there. This bug was most probably harmless, because
the TNC look-up probably failed for all padding and truncation nodes, but it
could also succeed and lead to various "interesting" errors.

This patch fixes the issue.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/gc.c |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)
Adrian Hunter - Aug. 7, 2010, 12:06 p.m.
Artem Bityutskiy wrote:
> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> 
> When moving nodes, do not try to look up truncation and padding nodes in TNC,
> because they do not exist there. This bug was most probably harmless, because
> the TNC look-up probably failed for all padding and truncation nodes, but it

The scan does not return padding nodes.

> could also succeed and lead to various "interesting" errors.

Why would it succeed?

> 
> This patch fixes the issue.
> 
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> ---
>  fs/ubifs/gc.c |   23 ++++++++++++++++++++---
>  1 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
> index fbb9272..f68e58d 100644
> --- a/fs/ubifs/gc.c
> +++ b/fs/ubifs/gc.c
> @@ -227,9 +227,26 @@ static int sort_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
>  	list_for_each_entry_safe(snod, tmp, &sleb->nodes, list) {
>  		int err;
>  
> -		ubifs_assert(snod->type != UBIFS_IDX_NODE);
> -		ubifs_assert(snod->type != UBIFS_REF_NODE);
> -		ubifs_assert(snod->type != UBIFS_CS_NODE);
> +		ubifs_assert(snod->type == UBIFS_INO_NODE  ||
> +			     snod->type == UBIFS_DATA_NODE ||
> +			     snod->type == UBIFS_DENT_NODE ||
> +			     snod->type == UBIFS_XENT_NODE ||
> +			     snod->type == UBIFS_TRUN_NODE ||
> +			     snod->type == UBIFS_PAD_NODE);
> +		ubifs_assert(key_type(c, &snod->key) == UBIFS_DATA_KEY ||
> +			     key_type(c, &snod->key) == UBIFS_INO_KEY  ||
> +			     key_type(c, &snod->key) == UBIFS_DENT_KEY ||
> +			     key_type(c, &snod->key) == UBIFS_XENT_KEY);
> +
> +		if (snod->type != UBIFS_INO_NODE  &&
> +		    snod->type != UBIFS_DATA_NODE &&
> +		    snod->type != UBIFS_DENT_NODE &&
> +		    snod->type != UBIFS_XENT_NODE) {
> +			/* Truncation or padding node, zap it */
> +			list_del(&snod->list);
> +			kfree(snod);
> +			continue;
> +		}
>  
>  		err = ubifs_tnc_has_node(c, &snod->key, 0, sleb->lnum,
>  					 snod->offs, 0);
Artem Bityutskiy - Aug. 7, 2010, 12:45 p.m.
Adrian, thanks for review!

On Sat, 2010-08-07 at 15:06 +0300, Adrian Hunter wrote:
> Artem Bityutskiy wrote:
> > From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> > 
> > When moving nodes, do not try to look up truncation and padding nodes in TNC,
> > because they do not exist there. This bug was most probably harmless, because
> > the TNC look-up probably failed for all padding and truncation nodes, but it
> 
> The scan does not return padding nodes.

Oh yes.

> > could also succeed and lead to various "interesting" errors.
> 
> Why would it succeed?

In 'ubifs_add_snod()' we call call 'key_read(c, &ino->key, &snod->key)'
for truncation nodes. However, 'struct ubifs_trun_node' does not have
key, so we read something else, and may happen to succeed in TNC lookup
then.

I guess the 'ubifs_add_snod()' should be fixed. Then key for truncation
nodes will be zero, and TNC lookup wont's succeed for sure, but it is
safer still to avoid look up for anything but data/ino/dent/xent nodes,
I think, no?

Patch

diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
index fbb9272..f68e58d 100644
--- a/fs/ubifs/gc.c
+++ b/fs/ubifs/gc.c
@@ -227,9 +227,26 @@  static int sort_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
 	list_for_each_entry_safe(snod, tmp, &sleb->nodes, list) {
 		int err;
 
-		ubifs_assert(snod->type != UBIFS_IDX_NODE);
-		ubifs_assert(snod->type != UBIFS_REF_NODE);
-		ubifs_assert(snod->type != UBIFS_CS_NODE);
+		ubifs_assert(snod->type == UBIFS_INO_NODE  ||
+			     snod->type == UBIFS_DATA_NODE ||
+			     snod->type == UBIFS_DENT_NODE ||
+			     snod->type == UBIFS_XENT_NODE ||
+			     snod->type == UBIFS_TRUN_NODE ||
+			     snod->type == UBIFS_PAD_NODE);
+		ubifs_assert(key_type(c, &snod->key) == UBIFS_DATA_KEY ||
+			     key_type(c, &snod->key) == UBIFS_INO_KEY  ||
+			     key_type(c, &snod->key) == UBIFS_DENT_KEY ||
+			     key_type(c, &snod->key) == UBIFS_XENT_KEY);
+
+		if (snod->type != UBIFS_INO_NODE  &&
+		    snod->type != UBIFS_DATA_NODE &&
+		    snod->type != UBIFS_DENT_NODE &&
+		    snod->type != UBIFS_XENT_NODE) {
+			/* Truncation or padding node, zap it */
+			list_del(&snod->list);
+			kfree(snod);
+			continue;
+		}
 
 		err = ubifs_tnc_has_node(c, &snod->key, 0, sleb->lnum,
 					 snod->offs, 0);