Messages in this thread | | | Date | Thu, 9 Mar 2017 16:04:39 +0900 | From | Hyunchul Lee <> | Subject | Re: [PATCH 2/4] ubifs: Fix unlink code wrt. double hash lookups |
| |
Richard,
this patch works well. but i found some trivial mistakes.
On Thu, Feb 09, 2017 at 10:28:35PM +0100, Richard Weinberger wrote: > When removing an encrypted file with a long anem and without having > the key we have to be able to locate and remove the directory entry > via a double hash. This corner case was simply forgotten. > > Reported-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at> > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > fs/ubifs/journal.c | 10 ++++- > fs/ubifs/tnc.c | 129 ++++++++++++++++++++++++++++++++++++++++++++--------- > fs/ubifs/ubifs.h | 2 + > 3 files changed, 117 insertions(+), 24 deletions(-) > > diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c > index f3b620cbdda4..7aef413ea2a9 100644 > --- a/fs/ubifs/journal.c > +++ b/fs/ubifs/journal.c > @@ -585,7 +585,10 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir, > > if (!xent) { > dent->ch.node_type = UBIFS_DENT_NODE; > - dent_key_init(c, &dent_key, dir->i_ino, nm); > + if (nm->hash) > + dent_key_init_hash(c, &dent_key, dir->i_ino, nm->hash); > + else > + dent_key_init(c, &dent_key, dir->i_ino, nm); > } else { > dent->ch.node_type = UBIFS_XENT_NODE; > xent_key_init(c, &dent_key, dir->i_ino, nm); > @@ -629,7 +632,10 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir, > kfree(dent); > > if (deletion) { > - err = ubifs_tnc_remove_nm(c, &dent_key, nm); > + if (nm->hash) > + err = ubifs_tnc_remove_dh(c, &dent_key, nm->minor_hash); > + else > + err = ubifs_tnc_remove_nm(c, &dent_key, nm); > if (err) > goto out_ro; > err = ubifs_add_dirt(c, lnum, dlen); > diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c > index 709aa098dd46..d84f4ba467a3 100644 > --- a/fs/ubifs/tnc.c > +++ b/fs/ubifs/tnc.c > @@ -1880,48 +1880,65 @@ int ubifs_tnc_lookup_nm(struct ubifs_info *c, const union ubifs_key *key, > return do_lookup_nm(c, key, node, nm); > } > > -static int do_lookup_dh(struct ubifs_info *c, const union ubifs_key *key, > - struct ubifs_dent_node *dent, uint32_t cookie) > +static int search_dh_cookie(struct ubifs_info *c, const union ubifs_key *key, > + struct ubifs_dent_node *dent, uint32_t cookie, > + struct ubifs_znode **zn, int *n) > { > - int n, err, type = key_type(c, key); > - struct ubifs_znode *znode; > + int err;
i guess that err should be initialized with -ENOENT to avoid the first call of tnc_next(c, &znode, n).
> + struct ubifs_znode *znode = *zn; > struct ubifs_zbranch *zbr; > - union ubifs_key *dkey, start_key; > - > - ubifs_assert(is_hash_key(c, key)); > - > - lowest_dent_key(c, &start_key, key_inum(c, key)); > - > - mutex_lock(&c->tnc_mutex); > - err = ubifs_lookup_level0(c, &start_key, &znode, &n); > - if (unlikely(err < 0)) > - goto out_unlock; > + union ubifs_key *dkey; > > for (;;) { > if (!err) { > - err = tnc_next(c, &znode, &n); > + err = tnc_next(c, &znode, n); > if (err) > - goto out_unlock; > + goto out; > } > > - zbr = &znode->zbranch[n]; > + zbr = &znode->zbranch[*n]; > dkey = &zbr->key; > > if (key_inum(c, dkey) != key_inum(c, key) || > - key_type(c, dkey) != type) { > + key_type(c, dkey) != key_type(c, key)) { > err = -ENOENT; > - goto out_unlock; > + goto out; > } > > err = tnc_read_hashed_node(c, zbr, dent); > if (err) > - goto out_unlock; > + goto out; > > if (key_hash(c, key) == key_hash(c, dkey) && > - le32_to_cpu(dent->cookie) == cookie) > - goto out_unlock; > + le32_to_cpu(dent->cookie) == cookie) { > + *zn = znode; > + goto out; > + } > } > > +out: > + > + return err; > +} > + > +static int do_lookup_dh(struct ubifs_info *c, const union ubifs_key *key, > + struct ubifs_dent_node *dent, uint32_t cookie) > +{ > + int n, err; > + struct ubifs_znode *znode; > + union ubifs_key start_key; > + > + ubifs_assert(is_hash_key(c, key)); > + > + lowest_dent_key(c, &start_key, key_inum(c, key)); > + > + mutex_lock(&c->tnc_mutex); > + err = ubifs_lookup_level0(c, &start_key, &znode, &n); > + if (unlikely(err < 0)) > + goto out_unlock; > + > + err = search_dh_cookie(c, key, dent, cookie, &znode, &n); > + > out_unlock: > mutex_unlock(&c->tnc_mutex); > return err; > @@ -2663,6 +2680,74 @@ int ubifs_tnc_remove_nm(struct ubifs_info *c, const union ubifs_key *key, > } > > /** > + * ubifs_tnc_remove_dh - remove an index entry for a "double hashed" node. > + * @c: UBIFS file-system description object > + * @key: key of node > + * @cookie: node cookie for collision resolution > + * > + * Returns %0 on success or negative error code on failure. > + */ > +int ubifs_tnc_remove_dh(struct ubifs_info *c, const union ubifs_key *key, > + uint32_t cookie) > +{ > + int n, err; > + struct ubifs_znode *znode; > + struct ubifs_dent_node *dent; > + struct ubifs_zbranch *zbr; > + > + if (!c->double_hash) > + return -EOPNOTSUPP; > + > + mutex_lock(&c->tnc_mutex); > + err = lookup_level0_dirty(c, key, &znode, &n); > + if (err <= 0) > + goto out_unlock; > + > + zbr = &znode->zbranch[n]; > + dent = kmalloc(UBIFS_MAX_DENT_NODE_SZ, GFP_NOFS); > + if (!dent) { > + err = -ENOMEM; > + goto out_unlock; > + } > + > + err = tnc_read_hashed_node(c, zbr, dent); > + if (err) > + goto out_free; > + > + /* If the cookie does not match, we're facing a hash collision. */ > + if (le32_to_cpu(dent->cookie) != cookie) { > + union ubifs_key start_key; > + > + lowest_dent_key(c, &start_key, key_inum(c, key)); > + > + err = ubifs_lookup_level0(c, &start_key, &znode, &n); > + if (unlikely(err < 0)) > + goto out_unlock;
i guess that out_unlock should be replaced with out_free to free dent.
> + > + err = search_dh_cookie(c, key, dent, cookie, &znode, &n); > + if (err) > + goto out_free; > + } > + > + if (znode->cnext || !ubifs_zn_dirty(znode)) { > + znode = dirty_cow_bottom_up(c, znode); > + if (IS_ERR(znode)) { > + err = PTR_ERR(znode); > + goto out_unlock;
this out_unlock should also be.
> + } > + } > + err = tnc_delete(c, znode, n); > + > +out_free: > + kfree(dent); > +out_unlock: > + if (!err) > + err = dbg_check_tnc(c, 0); > + mutex_unlock(&c->tnc_mutex); > + return err; > +} > + > +/** > * key_in_range - determine if a key falls within a range of keys. > * @c: UBIFS file-system description object > * @key: key to check > diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h > index ca72382ce6cc..36df4613b803 100644 > --- a/fs/ubifs/ubifs.h > +++ b/fs/ubifs/ubifs.h > @@ -1589,6 +1589,8 @@ int ubifs_tnc_add_nm(struct ubifs_info *c, const union ubifs_key *key, > int ubifs_tnc_remove(struct ubifs_info *c, const union ubifs_key *key); > int ubifs_tnc_remove_nm(struct ubifs_info *c, const union ubifs_key *key, > const struct fscrypt_name *nm); > +int ubifs_tnc_remove_dh(struct ubifs_info *c, const union ubifs_key *key, > + uint32_t cookie); > int ubifs_tnc_remove_range(struct ubifs_info *c, union ubifs_key *from_key, > union ubifs_key *to_key); > int ubifs_tnc_remove_ino(struct ubifs_info *c, ino_t inum); > -- > 2.10.2 > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
thanks, Hyunchul
| |