Re: [PATCH] fs: Fix uninitialized value issue in from_kuid

[Jan Kara]

On Thu 17-10-24 11:22:17, Alessandro Zanni wrote:
> On 24/10/16 04:52, Christian Brauner wrote:
> > On Wed, Oct 16, 2024 at 03:23:39PM +0200, Jan Kara wrote:
> > > That being said there are many more places calling notify_change() and none
> > > of them is doing the initialization so this patch only fixes that one
> > > particular syzbot reproducer but doesn't really deal with the problem.
> > > Looking at the bigger picture I think the right solution really is to fix
> > > ocfs2_setattr() to not touch attr->ia_uid when ATTR_UID isn't set and
> > > similarly for attr->ia_gid and ATTR_GID.
> > 
> > Yes, that's what we did for similar bugs.
> 
> Thanks for the valuable comments.
> 
> I digged more into the code. I think the two possible fixes are: 
> i) return 0 from ocfs2_setattr() if ATTR_UID/ATTR_GID are not set
> ii) enter in trace_ocfs2_setattr() only if ATTR_UID/ATTR_GID are set
> 
> What do you think?

I think the easiest fix is like:

        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);

Bonus points for fixing up overly long lines I have in my proposal :)

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR