[PATCH v4 11/14] Add start_renaming_two_dentries()

[NeilBrown]

On Thu, 30 Oct 2025, Al Viro wrote:
> On Thu, Oct 30, 2025 at 10:31:11AM +1100, NeilBrown wrote:
> 
> > +++ b/fs/debugfs/inode.c
> 
> Why does debugfs_change_name() need any of that horror?  Seriously, WTF?
> This is strictly a name change on a filesystem that never, ever moves
> anything from one directory to another.

"horror" is clearly in the eye of the beholder, and not a helpful
description...

Is there anything in this use of start_renaming_two_dentries() which is
harmful?  I agree that not all of the functionality is needed in this
case, but some of it is.

Would you prefer we also add
   start_renaming_two_dentries_with_same_parent()
or similar?

> 
> IMO struct renamedata is a fucking eyesore, but that aside, this:
> 
> > @@ -539,22 +540,30 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi,
> >  	if (ret)
> >  		goto out;
> >  
> > -	lock_rename(tmp_parent, fsi->sb->s_root);
> > +	rd.old_parent = tmp_parent;
> > +	rd.new_parent = fsi->sb->s_root;
> >  
> >  	/* booleans */
> > -	d_exchange(tmp_bool_dir, fsi->bool_dir);
> > +	ret = start_renaming_two_dentries(&rd, tmp_bool_dir, fsi->bool_dir);
> > +	if (!ret) {
> > +		d_exchange(tmp_bool_dir, fsi->bool_dir);
> >  
> > -	swap(fsi->bool_num, bool_num);
> > -	swap(fsi->bool_pending_names, bool_names);
> > -	swap(fsi->bool_pending_values, bool_values);
> > +		swap(fsi->bool_num, bool_num);
> > +		swap(fsi->bool_pending_names, bool_names);
> > +		swap(fsi->bool_pending_values, bool_values);
> >  
> > -	fsi->bool_dir = tmp_bool_dir;
> > +		fsi->bool_dir = tmp_bool_dir;
> > +		end_renaming(&rd);
> > +	}
> >  
> >  	/* classes */
> > -	d_exchange(tmp_class_dir, fsi->class_dir);
> > -	fsi->class_dir = tmp_class_dir;
> > +	ret = start_renaming_two_dentries(&rd, tmp_class_dir, fsi->class_dir);
> > +	if (ret == 0) {
> > +		d_exchange(tmp_class_dir, fsi->class_dir);
> > +		fsi->class_dir = tmp_class_dir;
> >  
> > -	unlock_rename(tmp_parent, fsi->sb->s_root);
> > +		end_renaming(&rd);
> > +	}
> >  
> >  out:
> >  	sel_remove_old_bool_data(bool_num, bool_names, bool_values);
> 
> is very interesting - suddenly you get two non-overlapping scopes instead of one.
> Why is that OK?
> 

>From the perspective of code performing lookup of these names, two
consecutive lookups would not be locked so they could see
inconsistencies anyway.
>From the perspective of code changing these names, that is protected by
selinux_state.policy_mutex which is held across the combined operation.
A readdir could possibly see the old inum for one name and the new inum
for the other name.  I don't imagine this would be a problem.

I have added a comment to the commit message to highlight this.

Thanks,
NeilBrown