linux-kernel - Re: [PATCH v2] fs: Fix uninitialized value issue in from_kuid and from

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241017130610.sryqcth6e2yao3gx@quack3>
Date: Thu, 17 Oct 2024 15:06:10 +0200
From: Jan Kara <jack@...e.cz>
To: Alessandro Zanni <alessandro.zanni87@...il.com>
Cc: viro@...iv.linux.org.uk, brauner@...nel.org, jack@...e.cz,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	skhan@...uxfoundation.org, anupnewsmail@...il.com,
	alessandrozanni.dev@...il.com,
	syzbot+6c55f725d1bdc8c52058@...kaller.appspotmail.com
Subject: Re: [PATCH v2] fs: Fix uninitialized value issue in from_kuid and
 from_kgid

On Thu 17-10-24 14:05:51, Alessandro Zanni wrote:
> ocfs2_setattr() uses attr->ia_mode, attr->ia_uid and attr->ia_gid in
> a trace point even though ATTR_MODE, ATTR_UID and ATTR_GID aren't set.
> 
> Initialize all fields of newattrs to avoid uninitialized variables, by
> checking if ATTR_MODE, ATTR_UID, ATTR_GID are initialized, otherwise 0.
> 
> Reported-by: syzbot+6c55f725d1bdc8c52058@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6c55f725d1bdc8c52058
> Signed-off-by: Alessandro Zanni <alessandro.zanni87@...il.com>

Thanks! The patch looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> Notes:
>     v2: fix ocfs2_setattr to avoid similar issues; improved commit description
> 
>  fs/ocfs2/file.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index ad131a2fc58e..58887456e3c5 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1129,9 +1129,12 @@ int ocfs2_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  	trace_ocfs2_setattr(inode, dentry,
>  			    (unsigned long long)OCFS2_I(inode)->ip_blkno,
>  			    dentry->d_name.len, dentry->d_name.name,
> -			    attr->ia_valid, attr->ia_mode,
> -			    from_kuid(&init_user_ns, attr->ia_uid),
> -			    from_kgid(&init_user_ns, attr->ia_gid));
> +			    attr->ia_valid,
> +				attr->ia_valid & ATTR_MODE ? attr->ia_mode : 0,
> +				attr->ia_valid & ATTR_UID ?
> +					from_kuid(&init_user_ns, attr->ia_uid) : 0,
> +				attr->ia_valid & ATTR_GID ?
> +					from_kgid(&init_user_ns, attr->ia_gid) : 0);
>  
>  	/* ensuring we don't even attempt to truncate a symlink */
>  	if (S_ISLNK(inode->i_mode))
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR