linux-kernel - Re: [PATCH 1/2] jffs2: initialize filesystem-private inode info in ->alloc

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <77684009a0a2c527168df4c40077e51a69773ee4.camel@infinera.com>
Date: Thu, 9 Jan 2025 10:12:16 +0000
From: Joakim Tjernlund <Joakim.Tjernlund@...inera.com>
To: "chengzhihao1@...wei.com" <chengzhihao1@...wei.com>, "pchelkin@...ras.ru"
	<pchelkin@...ras.ru>, "richard@....at" <richard@....at>
CC: "dwmw2@...radead.org" <dwmw2@...radead.org>, "yang.tao172@....com.cn"
	<yang.tao172@....com.cn>, "lu.zhongjun@....com.cn" <lu.zhongjun@....com.cn>,
	"wang.yong12@....com.cn" <wang.yong12@....com.cn>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"lvc-project@...uxtesting.org" <lvc-project@...uxtesting.org>,
	"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH 1/2] jffs2: initialize filesystem-private inode info in
 ->alloc_inode callback

On Sun, 2024-11-17 at 21:44 +0300, Fedor Pchelkin wrote:
> The symlink body (->target) should be freed at the same time as the inode
> itself per commit 4fdcfab5b553 ("jffs2: fix use-after-free on symlink
> traversal"). It is a filesystem-specific field but there exist several
> error paths during generic inode allocation when ->free_inode(), namely
> jffs2_free_inode(), is called with still uninitialized private info.
> 
> The calltrace looks like:
>  alloc_inode
>   inode_init_always // fails
>    i_callback
>     free_inode
>     jffs2_free_inode // touches uninit ->target field
> 
> Commit af9a8730ddb6 ("jffs2: Fix potential illegal address access in
> jffs2_free_inode") approached the observed problem but fixed it only
> partially. Our local Syzkaller instance is still hitting these kinds of
> failures.
> 
> The thing is that jffs2_i_init_once(), where the initialization of
> f->target has been moved, is called once per slab allocation so it won't
> be called for the object structure possibly retrieved later from the slab
> cache for reuse.
> 
> The practice followed by many other filesystems is to initialize
> filesystem-private inode contents in the corresponding ->alloc_inode()
> callbacks. This also allows to drop initialization from jffs2_iget() and
> jffs2_new_inode() as ->alloc_inode() is called in those places.
> 
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> 
> Fixes: 4fdcfab5b553 ("jffs2: fix use-after-free on symlink traversal")
> Cc: stable@...r.kernel.org
> Signed-off-by: Fedor Pchelkin <pchelkin@...ras.ru>
> ---
>  fs/jffs2/fs.c    | 2 --
>  fs/jffs2/super.c | 3 ++-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
> index d175cccb7c55..85c4b273918f 100644
> --- a/fs/jffs2/fs.c
> +++ b/fs/jffs2/fs.c
> @@ -271,7 +271,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);
> @@ -439,7 +438,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 4545f885c41e..b56ff63357f3 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 = alloc_inode_sb(sb, jffs2_inode_cachep, GFP_KERNEL);
>  	if (!f)
>  		return NULL;
> +
> +	jffs2_init_inode_info(f);
>  	return &f->vfs_inode;
>  }
>  
> @@ -58,7 +60,6 @@ static void jffs2_i_init_once(void *foo)
>  	struct jffs2_inode_info *f = foo;
>  
>  	mutex_init(&f->sem);
> -	f->target = NULL;
>  	inode_init_once(&f->vfs_inode);
>  }
>  

Has this one been lost?