| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d2697496-c7db-94a1-4292-03625b47cf63@huawei.com>
Date: Sat, 16 Feb 2019 16:53:35 +0800
From: yuyufen <yuyufen@...wei.com>
To: <dwmw2@...radead.org>, <richard.weinberger@...il.com>,
<linux-mtd@...ts.infradead.org>
CC: <Joakim.Tjernlund@...inera.com>, <houtao1@...wei.com>,
<viro@...iv.linux.org.uk>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] jffs2: safely remove obsolete dirent from the f->dents
list
ping?
On 2019/1/23 15:22, Yufen Yu wrote:
> We may delete a bunch of directory entries, operating such as:
> getdents(), unlink(), getdents()..., until the end of the directory.
> jffs2 handles f_pos on the directory merely as the position on the
> f->dents list. So, the next getdents() may skip some entries
> before f_pos, if we remove some entries from the list between two
> getdents(), resulting in some entries of the directory cannot be deleted.
>
> Commit 15953580e79b is introduced to resolve this bug by not
> removing delete entries from the list immediately.
>
> However, when CONFIG_JFFS2_SUMMARY is not set, it can cause the
> following issues:
>
> * 'deletion' dirents is always in the f->dents list, wasting memory
> resource. For example:
> There is a file named 'file1'. Then we rename it:
> mv file1 file2;
> mv file2 file3;
> ...
> mv file99999 file1000000
>
> All of file1~file1000000 dirents always in the f->dents list.
>
> * Though, the list we're traversing is already ordered by CRC,
> it could waste much CPU time when the list is very long.
>
> To fix, we define two variables in struct jffs2_inode_info: nr_dir_opening,
> obsolete_count, and two new functions: jffs2_dir_open(), jffs2_dir_release().
>
> When open a directory, jffs2_dir_open() will increase nr_dir_opening,
> which will be decreased by jffs2_dir_release(). if the value is 0,
> it means nobody open the dir and nobody in getdents()/seek() semantics.
> Thus, we can remove all obsolete dirent from the list.
>
> When delete a file, jffs2_do_unlink() can remove the dirent directly if
> nobody open the directory(i.e. nr_dir_opending == 0). Otherwise, it just
> increase obsolete_count, denoting obsolete dirent count of the list.
>
> Fixes: 15953580e79b ("[JFFS2] Improve getdents vs. f_pos handling on NOR flash.")
> Signed-off-by: Yufen Yu <yuyufen@...wei.com>
> ---
> fs/jffs2/dir.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/jffs2/jffs2_fs_i.h | 7 +++++++
> fs/jffs2/super.c | 4 ++++
> fs/jffs2/write.c | 30 +++++++++++++++++++---------
> include/uapi/linux/jffs2.h | 4 ++++
> 5 files changed, 85 insertions(+), 9 deletions(-)
>
> diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
> index f20cff1194bb..aed872dcd0ca 100644
> --- a/fs/jffs2/dir.c
> +++ b/fs/jffs2/dir.c
> @@ -37,6 +37,8 @@ static int jffs2_mknod (struct inode *,struct dentry *,umode_t,dev_t);
> static int jffs2_rename (struct inode *, struct dentry *,
> struct inode *, struct dentry *,
> unsigned int);
> +static int jffs2_dir_release (struct inode *, struct file *);
> +static int jffs2_dir_open (struct inode *, struct file *);
>
> const struct file_operations jffs2_dir_operations =
> {
> @@ -45,6 +47,8 @@ const struct file_operations jffs2_dir_operations =
> .unlocked_ioctl=jffs2_ioctl,
> .fsync = jffs2_fsync,
> .llseek = generic_file_llseek,
> + .open = jffs2_dir_open,
> + .release = jffs2_dir_release,
> };
>
>
> @@ -865,3 +869,48 @@ static int jffs2_rename (struct inode *old_dir_i, struct dentry *old_dentry,
> return 0;
> }
>
> +static int jffs2_dir_open(struct inode *dir_i, struct file *filp)
> +{
> +#ifndef CONFIG_JFFS2_SUMMARY
> + struct jffs2_inode_info *dir_f = JFFS2_INODE_INFO(dir_i);
> + atomic_inc(&dir_f->nr_dir_opening);
> +#endif
> +
> + return 0;
> +}
> +
> +static int jffs2_dir_release(struct inode *dir_i, struct file *filp)
> +{
> +#ifndef CONFIG_JFFS2_SUMMARY
> + struct jffs2_inode_info *dir_f = JFFS2_INODE_INFO(dir_i);
> +
> + BUG_ON(atomic_read(&dir_f->nr_dir_opening) <= 0);
> +
> + mutex_lock(&dir_f->sem);
> + /* jffs2_dir_open may increase nr_dir_opening after
> + * atomic_dec_and_test() returning true.
> + * However, it cannot traverse the list until hold
> + * mutex dir_f->sem lock, so that we can go on
> + * removing.*/
> + if (atomic_dec_and_test(&dir_f->nr_dir_opening) &&
> + dir_f->obsolete_count > JFFS2_OBS_DIRENT_LIMIT) {
> + struct jffs2_full_dirent **prev = &dir_f->dents;
> +
> + /* remove all obsolete dirent from the list, which
> + * can save memory space and reduce CPU time for
> + * traverse the list */
> + while(*prev) {
> + if ((*prev)->raw == NULL && (*prev)->ino == 0) {
> + struct jffs2_full_dirent *this = *prev;
> + *prev = this->next;
> + jffs2_free_full_dirent(this);
> + } else
> + prev = &((*prev)->next);
> + }
> + dir_f->obsolete_count = 0;
> + }
> + mutex_unlock(&dir_f->sem);
> +#endif
> +
> + return 0;
> +}
> diff --git a/fs/jffs2/jffs2_fs_i.h b/fs/jffs2/jffs2_fs_i.h
> index 2e4a86763c07..a4da25d16cb4 100644
> --- a/fs/jffs2/jffs2_fs_i.h
> +++ b/fs/jffs2/jffs2_fs_i.h
> @@ -50,6 +50,13 @@ struct jffs2_inode_info {
>
> uint16_t flags;
> uint8_t usercompr;
> +
> + /* obsolete dirent count in the list of 'dents' */
> + uint8_t obsolete_count;
> +
> + /* Directory open refcount */
> + atomic_t nr_dir_opening;
> +
> struct inode vfs_inode;
> };
>
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 87bdf0f4cba1..bf181b0ade9c 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -41,6 +41,10 @@ static struct inode *jffs2_alloc_inode(struct super_block *sb)
> f = kmem_cache_alloc(jffs2_inode_cachep, GFP_KERNEL);
> if (!f)
> return NULL;
> +
> + atomic_set(&f->nr_dir_opening, 0);
> + f->obsolete_count = 0;
> +
> return &f->vfs_inode;
> }
>
> diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
> index cda9a361368e..780b4fd9af51 100644
> --- a/fs/jffs2/write.c
> +++ b/fs/jffs2/write.c
> @@ -600,29 +600,41 @@ int jffs2_do_unlink(struct jffs2_sb_info *c, struct jffs2_inode_info *dir_f,
> } else {
> uint32_t nhash = full_name_hash(NULL, name, namelen);
>
> - fd = dir_f->dents;
> + struct jffs2_full_dirent **prev = &dir_f->dents;
> +
> /* We don't actually want to reserve any space, but we do
> want to be holding the alloc_sem when we write to flash */
> mutex_lock(&c->alloc_sem);
> mutex_lock(&dir_f->sem);
>
> - for (fd = dir_f->dents; fd; fd = fd->next) {
> - if (fd->nhash == nhash &&
> - !memcmp(fd->name, name, namelen) &&
> - !fd->name[namelen]) {
> + while((*prev) && (*prev)->nhash <= nhash) {
> + if ((*prev)->nhash == nhash &&
> + !memcmp((*prev)->name, name, namelen) &&
> + !(*prev)->name[namelen]) {
>
> + fd = *prev;
> jffs2_dbg(1, "Marking old dirent node (ino #%u) @%08x obsolete\n",
> fd->ino, ref_offset(fd->raw));
> jffs2_mark_node_obsolete(c, fd->raw);
> - /* We don't want to remove it from the list immediately,
> - because that screws up getdents()/seek() semantics even
> - more than they're screwed already. Turn it into a
> - node-less deletion dirent instead -- a placeholder */
> fd->raw = NULL;
> fd->ino = 0;
> +
> + /* if nr_dir_openning is 0, we think nobody open the dir, and
> + * nobody being in getdents()/seek() semantics. Thus, we can
> + * safely remove this obsolete dirent from the list. Otherwise,
> + * we just increase obsolete_count, and finally delete it in
> + * jffs2_dir_release() */
> + if (atomic_read(&dir_f->nr_dir_opening) == 0) {
> + *prev = fd->next;
> + jffs2_free_full_dirent(fd);
> + } else
> + dir_f->obsolete_count++;
> +
> break;
> }
> + prev = &((*prev)->next);
> }
> +
> mutex_unlock(&dir_f->sem);
> }
>
> diff --git a/include/uapi/linux/jffs2.h b/include/uapi/linux/jffs2.h
> index a18b719f49d4..dff3ac2d6b0c 100644
> --- a/include/uapi/linux/jffs2.h
> +++ b/include/uapi/linux/jffs2.h
> @@ -35,6 +35,10 @@
> */
> #define JFFS2_MAX_NAME_LEN 254
>
> +/* The obsolete dirent of dents list limit. When the number over
> + * this limit, we can remove the obsoleted dents. */
> +#define JFFS2_OBS_DIRENT_LIMIT 64
> +
> /* How small can we sensibly write nodes? */
> #define JFFS2_MIN_DATA_LEN 128
>
Powered by blists - more mailing lists