| Message ID | 20200106080411.41394-1-yi.zhang@huawei.com |
|---|---|
| State | New |
| Delegated to: | David Woodhouse |
| Headers | show |
| Series | jffs2: move jffs2_init_inode_info() just after allocating inode | expand |
ping. On 2020/1/6 16:04, zhangyi (F) wrote: > After commit 4fdcfab5b553 ("jffs2: fix use-after-free on symlink > traversal"), it expose a freeing uninitialized memory problem due to > this commit move the operaion of freeing f->target to > jffs2_i_callback(), which may not be initialized in some error path of > allocating jffs2 inode (eg: jffs2_iget()->iget_locked()-> > destroy_inode()->..->jffs2_i_callback()->kfree(f->target)). > > Fix this by initialize the jffs2_inode_info just after allocating it. > > Reported-by: Guohua Zhong <zhongguohua1@huawei.com> > Reported-by: Huaijie Yi <yihuaijie@huawei.com> > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> > Cc: stable@vger.kernel.org > --- > fs/jffs2/fs.c | 2 -- > fs/jffs2/super.c | 2 ++ > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c > index ab8cdd9e9325..50a9df7d43a5 100644 > --- a/fs/jffs2/fs.c > +++ b/fs/jffs2/fs.c > @@ -270,7 +270,6 @@ struct inode *jffs2_iget(struct super_block *sb, unsigned long ino) > f = JFFS2_INODE_INFO(inode); > c = JFFS2_SB_INFO(inode->i_sb); > > - jffs2_init_inode_info(f); > mutex_lock(&f->sem); > > ret = jffs2_do_read_inode(c, f, inode->i_ino, &latest_node); > @@ -438,7 +437,6 @@ struct inode *jffs2_new_inode (struct inode *dir_i, umode_t mode, struct jffs2_r > return ERR_PTR(-ENOMEM); > > f = JFFS2_INODE_INFO(inode); > - jffs2_init_inode_info(f); > mutex_lock(&f->sem); > > memset(ri, 0, sizeof(*ri)); > diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c > index 0e6406c4f362..90373898587f 100644 > --- a/fs/jffs2/super.c > +++ b/fs/jffs2/super.c > @@ -42,6 +42,8 @@ static struct inode *jffs2_alloc_inode(struct super_block *sb) > f = kmem_cache_alloc(jffs2_inode_cachep, GFP_KERNEL); > if (!f) > return NULL; > + > + jffs2_init_inode_info(f); > return &f->vfs_inode; > } > >
On 2020/1/6 16:04, zhangyi (F) wrote: > After commit 4fdcfab5b553 ("jffs2: fix use-after-free on symlink > traversal"), it expose a freeing uninitialized memory problem due to > this commit move the operaion of freeing f->target to > jffs2_i_callback(), which may not be initialized in some error path of > allocating jffs2 inode (eg: jffs2_iget()->iget_locked()-> > destroy_inode()->..->jffs2_i_callback()->kfree(f->target)). > > Fix this by initialize the jffs2_inode_info just after allocating it. We are having the same problem. After commit 4fdcfab5b553 ("jffs2: fix use-after-free on symlink > traversal"), f->target is freed before it is initialized in the iget_locked() path. This is dangerous and may trigger slub BUG_ON: kernel BUG at mm/slub.c:3824! Internal error: Oops - BUG: 0 [#1] SMP ARM CPU: 2 PID: 9 Comm: rcuos/0 Tainted: P O 4.4.185 #1 task: cf4a3f68 task.stack: cf4ca000 PC is at kfree+0xfc/0x264 LR is at jffs2_i_callback+0x10/0x28 [jffs2] pc : [<c032a4f0>] lr : [<bf0ab188>] psr: 400e0213 sp : cf4cbec8 ip : 00000000 fp : c0273df8 r10: ceb12848 r9 : 0000000c r8 : cdd52000 r7 : bf0ab188 r6 : 0000000c r5 : e7fddef0 r4 : c1121ba0 r3 : 00000100 r2 : c0ac4010 r1 : 00000002 r0 : e7fddef0 Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 32c5387d Table: 0e315940 DAC: 55555555 Process rcuos/0 (pid: 9, stack limit = 0xcf4ca190) Stack: (0xcf4cbec8 to 0xcf4cc000) bec0: c086efa8 c032a3a8 00000001 c0273e9c c0a29214 c3931db8 bee0: 00000000 0000000c ffffe000 cdd52000 0000000c ceb12848 c0273df8 bf0ab188 bf00: c0adf980 c0273e9c c0adf980 00000001 00000000 ffffff7c 00000000 cf4a3f68 bf20: c025bc18 cf4cbf24 cf4cbf24 c0a22448 c0adf980 cf4ca000 cf485ac0 00000000 bf40: c0adf980 c02739b0 00000000 00000000 00000000 c02380bc 00000000 c0adf380 bf60: c0adf980 00000000 00000000 00000000 00008001 cf4cbf74 cf4cbf74 00000000 bf80: 00000000 00000000 00008001 cf4cbf8c cf4cbf8c c0a22448 cf485ac0 c0237fb8 bfa0: 00000000 00000000 00000000 c0202db4 00000000 00000000 00000000 00000000 bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [<c032a4f0>] (kfree) from [<bf0ab188>] (jffs2_i_callback+0x10/0x28 [jffs2]) [<bf0ab188>] (jffs2_i_callback [jffs2]) from [<c0273e9c>] (rcu_nocb_kthread+0x4ec/0x504) [<c0273e9c>] (rcu_nocb_kthread) from [<c02380bc>] (kthread+0x104/0x118) [<c02380bc>] (kthread) from [<c0202db4>] (ret_from_fork+0x14/0x20) Code: 0300001a 143094e5 010013e3 0000001a (f201f0e7)
Hi, Cc +Richard +David On 2020/1/6 16:04, zhangyi (F) wrote: > After commit 4fdcfab5b553 ("jffs2: fix use-after-free on symlink > traversal"), it expose a freeing uninitialized memory problem due to > this commit move the operaion of freeing f->target to > jffs2_i_callback(), which may not be initialized in some error path of > allocating jffs2 inode (eg: jffs2_iget()->iget_locked()-> > destroy_inode()->..->jffs2_i_callback()->kfree(f->target)). > Could you please elaborate the scenario in which the use of a uninitialized f->target is possible ? IMO one case is that there are concurrent jffs2_lookup() and jffs2 GC on an evicted inode, and two new inodes are created, and then one needless inode is destroyed. > Fix this by initialize the jffs2_inode_info just after allocating it. > > Reported-by: Guohua Zhong <zhongguohua1@huawei.com> > Reported-by: Huaijie Yi <yihuaijie@huawei.com> > Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> > Cc: stable@vger.kernel.org > --- A Fixes tag is also needed here. > fs/jffs2/fs.c | 2 -- > fs/jffs2/super.c | 2 ++ > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c > index ab8cdd9e9325..50a9df7d43a5 100644 > --- a/fs/jffs2/fs.c > +++ b/fs/jffs2/fs.c > @@ -270,7 +270,6 @@ struct inode *jffs2_iget(struct super_block *sb, unsigned long ino) > f = JFFS2_INODE_INFO(inode); > c = JFFS2_SB_INFO(inode->i_sb); > > - jffs2_init_inode_info(f); > mutex_lock(&f->sem); > > ret = jffs2_do_read_inode(c, f, inode->i_ino, &latest_node); > @@ -438,7 +437,6 @@ struct inode *jffs2_new_inode (struct inode *dir_i, umode_t mode, struct jffs2_r > return ERR_PTR(-ENOMEM); > > f = JFFS2_INODE_INFO(inode); > - jffs2_init_inode_info(f); > mutex_lock(&f->sem); > > memset(ri, 0, sizeof(*ri)); > diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c > index 0e6406c4f362..90373898587f 100644 > --- a/fs/jffs2/super.c > +++ b/fs/jffs2/super.c > @@ -42,6 +42,8 @@ static struct inode *jffs2_alloc_inode(struct super_block *sb) > f = kmem_cache_alloc(jffs2_inode_cachep, GFP_KERNEL); > if (!f) > return NULL; > + > + jffs2_init_inode_info(f); > return &f->vfs_inode; > } > >
diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c index ab8cdd9e9325..50a9df7d43a5 100644 --- a/fs/jffs2/fs.c +++ b/fs/jffs2/fs.c @@ -270,7 +270,6 @@ struct inode *jffs2_iget(struct super_block *sb, unsigned long ino) f = JFFS2_INODE_INFO(inode); c = JFFS2_SB_INFO(inode->i_sb); - jffs2_init_inode_info(f); mutex_lock(&f->sem); ret = jffs2_do_read_inode(c, f, inode->i_ino, &latest_node); @@ -438,7 +437,6 @@ struct inode *jffs2_new_inode (struct inode *dir_i, umode_t mode, struct jffs2_r return ERR_PTR(-ENOMEM); f = JFFS2_INODE_INFO(inode); - jffs2_init_inode_info(f); mutex_lock(&f->sem); memset(ri, 0, sizeof(*ri)); diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c index 0e6406c4f362..90373898587f 100644 --- a/fs/jffs2/super.c +++ b/fs/jffs2/super.c @@ -42,6 +42,8 @@ static struct inode *jffs2_alloc_inode(struct super_block *sb) f = kmem_cache_alloc(jffs2_inode_cachep, GFP_KERNEL); if (!f) return NULL; + + jffs2_init_inode_info(f); return &f->vfs_inode; }
After commit 4fdcfab5b553 ("jffs2: fix use-after-free on symlink traversal"), it expose a freeing uninitialized memory problem due to this commit move the operaion of freeing f->target to jffs2_i_callback(), which may not be initialized in some error path of allocating jffs2 inode (eg: jffs2_iget()->iget_locked()-> destroy_inode()->..->jffs2_i_callback()->kfree(f->target)). Fix this by initialize the jffs2_inode_info just after allocating it. Reported-by: Guohua Zhong <zhongguohua1@huawei.com> Reported-by: Huaijie Yi <yihuaijie@huawei.com> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com> Cc: stable@vger.kernel.org --- fs/jffs2/fs.c | 2 -- fs/jffs2/super.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-)