| Message ID | 1281261452-9868-8-git-send-email-dedekind1@gmail.com |
|---|---|
| State | Accepted |
| Commit | 5b7a3a2e1b0cbc7d5410a8da60dac266a3e19268 |
| Headers | show |
Artem Bityutskiy wrote: > From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > > In the scanning code, in 'ubifs_add_snod()', we write rubbish into > 'snod->key', because we assume that on-flash truncation nodes have a key, but > they do not. If the other parts of UBIFS then mistakenly try to look-up > the truncation node key (they should not do this, but may do because of a bug), > we can succeed and corrupt TNC. It looks like we did have such a situation in > 'sort_nodes()' in gc.c. > > Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > --- > fs/ubifs/scan.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/fs/ubifs/scan.c b/fs/ubifs/scan.c > index 96c5253..a0a305c 100644 > --- a/fs/ubifs/scan.c > +++ b/fs/ubifs/scan.c > @@ -212,7 +212,6 @@ int ubifs_add_snod(const struct ubifs_info *c, struct ubifs_scan_leb *sleb, > case UBIFS_DENT_NODE: > case UBIFS_XENT_NODE: > case UBIFS_DATA_NODE: > - case UBIFS_TRUN_NODE: There is another bug in ubifs_recover_size_accum() that expects a truncate node to have a key. That would be fixed by using trun_key_init() here. > /* > * The key is in the same place in all keyed > * nodes.
Adrian Hunter wrote: > Artem Bityutskiy wrote: >> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> >> >> In the scanning code, in 'ubifs_add_snod()', we write rubbish into >> 'snod->key', because we assume that on-flash truncation nodes have a key, but >> they do not. If the other parts of UBIFS then mistakenly try to look-up >> the truncation node key (they should not do this, but may do because of a bug), >> we can succeed and corrupt TNC. It looks like we did have such a situation in >> 'sort_nodes()' in gc.c. >> >> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> >> --- >> fs/ubifs/scan.c | 1 - >> 1 files changed, 0 insertions(+), 1 deletions(-) >> >> diff --git a/fs/ubifs/scan.c b/fs/ubifs/scan.c >> index 96c5253..a0a305c 100644 >> --- a/fs/ubifs/scan.c >> +++ b/fs/ubifs/scan.c >> @@ -212,7 +212,6 @@ int ubifs_add_snod(const struct ubifs_info *c, struct ubifs_scan_leb *sleb, >> case UBIFS_DENT_NODE: >> case UBIFS_XENT_NODE: >> case UBIFS_DATA_NODE: >> - case UBIFS_TRUN_NODE: > > There is another bug in ubifs_recover_size_accum() that expects a truncate node > to have a key. That would be fixed by using trun_key_init() here. No I was wrong, the key in ubifs_recover_size_accum() is already created using trun_key_init() Still, trun_key_init might be better here because the all-zeroes key has a key-type of UBIFS_INO_KEY. What do you think? > >> /* >> * The key is in the same place in all keyed >> * nodes. > >
On Sun, 2010-08-08 at 14:08 +0300, Adrian Hunter wrote: > No I was wrong, the key in ubifs_recover_size_accum() is already created using > trun_key_init() > > Still, trun_key_init might be better here because the all-zeroes key has a key-type > of UBIFS_INO_KEY. What do you think? Did not really think, but off the top of my head it is probably good to initialize ->key to something invalid of all other nodes then.
diff --git a/fs/ubifs/scan.c b/fs/ubifs/scan.c index 96c5253..a0a305c 100644 --- a/fs/ubifs/scan.c +++ b/fs/ubifs/scan.c @@ -212,7 +212,6 @@ int ubifs_add_snod(const struct ubifs_info *c, struct ubifs_scan_leb *sleb, case UBIFS_DENT_NODE: case UBIFS_XENT_NODE: case UBIFS_DATA_NODE: - case UBIFS_TRUN_NODE: /* * The key is in the same place in all keyed * nodes.