Question about ext2

Question about ext2

[malfet]

Hi all!
I look up in implementation in ext2_rename and see the following statment:
       if (S_ISDIR(old_inode->i_mode)) {
                if (new_inode) {
                        retval = -ENOTEMPTY;
                        if (!empty_dir (new_inode))
                                goto end_rename;
                }
But I don't see any checkl like S_ISDIR on new_inode neither in ext2_rename neither in empty_dir. Is this bug? And, anyway, can I rename directory into not-empty file?
Thanks in advance,
	Nikita
P.S. Please, CC answer on malfet@mipt.sw.ru, because I'm not subscribed on this list

Re: Question about ext2

[Andrew Morton]

malfet@gw.mipt.sw.ru wrote:
> 
> Hi all!
> I look up in implementation in ext2_rename and see the following statment:
>        if (S_ISDIR(old_inode->i_mode)) {
>                 if (new_inode) {
>                         retval = -ENOTEMPTY;
>                         if (!empty_dir (new_inode))
>                                 goto end_rename;
>                 }
> But I don't see any checkl like S_ISDIR on new_inode neither in
> ext2_rename neither in empty_dir. Is this bug? And, anyway, can
> I rename directory into not-empty file?

It's implicit - rename can only rename files to files, and
directories to directories.  So the check is made at a higher
level.  Consequently when we get to ext2_rename, if we find
that the old inode is a directory, we *know* that the new
one is a directory as well.

I recently spent an hour decrypting this function.  Here is
a commented version which may prove helpful. It is from a
non-mainline branch of ext3, but it's much the same.


static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
			   struct inode * new_dir,struct dentry *new_dentry)
{
	handle_t *handle;

	/* old_inode is the thing we're renaming */
	struct inode * old_inode = old_dentry->d_inode;

	/* new_inode is what we're renaming it to (may be NULL) */
	struct inode * new_inode = new_dentry->d_inode;

	/* dir_bh is the buffer which contains old_inode's ".." entry */
	struct buffer_head * dir_bh = NULL;

	/* dir_de is the old_inode's ".." de.  Points into dir_bh->b_data */
	struct ext3_dir_entry_2 * dir_de = NULL;

	/* old_bh contains the old entry's de */
	struct buffer_head * old_bh;

	/* old_de points to the old entry's de, inside old_bh->b_data */
	struct ext3_dir_entry_2 * old_de;
	int err = -ENOENT;

	old_bh = NULL;

	handle = ext3_journal_start(old_dir, 2 * EXT3_DATA_TRANS_BLOCKS + 2);
	if (IS_ERR(handle))
		return PTR_ERR(handle);

	/* Find the current directory entry's bh and de */
	old_bh = ext3_find_entry (old_dentry, &old_de);
	if (!old_bh)
		goto end_rename;

	if (S_ISDIR(old_inode->i_mode)) {
		/*
		 * If the thing we're renaming is a directory, we'll need to
		 * change its ".." to point to a different parent.  Go find
		 * the ".." directory entry
		 */
		err = -EIO;
		dir_de = ext3_dotdot(handle, old_inode, &dir_bh);
		if (!dir_de)
			goto end_rename;
	}

	if (new_inode) {
		/* We're overwriting another object */
		struct buffer_head * new_bh;
		struct ext3_dir_entry_2 * new_de;

		/* If the renamee is a dir, then the victim MUST be a dir.
		 * It must not have any entries */
		err = -ENOTEMPTY;
		if (dir_de && !empty_dir (new_inode))
			goto out_dir;

		/* Go find the buffer and de for the victim */
		err = -ENOENT;
		new_bh = ext3_find_entry (new_dentry, &new_de);
		if (!new_bh)
			goto out_dir;

		/* Temporarily bump the renamee's link count.  Dunno why */
		ext3_inc_count(handle, old_inode);

		/* Overwrite the victim's directory info */
		ext3_set_link(handle, new_dir, new_de, new_bh, old_inode);
		new_inode->i_ctime = CURRENT_TIME;

		/* If renamee is a dir then the victim is a dir.  Drop nlink
		 * to account for the "." entry */
		if (dir_de)
			new_inode->i_nlink--;

		/* victim loses a refcount.  If it is a dir, it will be
		 * removed altogether, in do_rename->dput->iput */
		ext3_dec_count(handle, new_inode);

		/* Add an orphan record into this transaction.  If the victim
		 * dir is huge, iput's truncate may cross multiple transactions.
		 * We remove the orphan record inside the transaction which
		 * actually releases the inode */
		if (!new_inode->i_nlink)
			ext3_orphan_add(handle, new_inode);
	} else {
		/* The new name is not currently used */
		if (dir_de) {
			/* Moving a directory will add an extra ref to its
			 * parent because of the ".." entry */
			err = -EMLINK;
			if (new_dir->i_nlink >= EXT3_LINK_MAX)
				goto out_dir;
		}

		/* Temporarily bump the renamee's link count.  Dunno why */
		ext3_inc_count(handle, old_inode);

		/* Add the renamee to its new directory */
		err = ext3_add_entry (handle, new_dentry, old_inode);
		if (err) {
			ext3_dec_count(handle, old_inode);
			goto out_dir;
		}

		/* If we just moved a directory, parent gets another ref for
		 * ".." */
		if (dir_de)
			ext3_inc_count(handle, new_dir);
	}

	/* Remove the renamee's old directory entry */
	ext3_delete_entry(handle, old_dir, old_de, old_bh);
	old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME;
	ext3_mark_inode_dirty(handle, old_dir);
	brelse (old_bh);
	old_inode->i_ctime = CURRENT_TIME;

	/* Drop the temp refcount.  Also marks the renamee's inode dirty */
	ext3_dec_count(handle, old_inode);

	if (dir_bh) {
		/* We moved a directory.  Make its ".." entry point to the new
		 * parent */
		ext3_set_link(handle, old_inode, dir_de, dir_bh, new_dir);

		/* The old parent no longer has the renamee's ".." pointing
		 * to it */
		ext3_dec_count(handle, old_dir);
	}

	ext3_journal_stop(handle, old_dir);
	return 0;

out_dir:
	brelse (dir_bh);
end_rename:
	brelse (old_bh);
	ext3_journal_stop(handle, old_dir);
	return err;
}

Re[2]: Question about ext2

[malfet]

> It's implicit - rename can only rename files to files, and
> directories to directories.  So the check is made at a higher
> level.  Consequently when we get to ext2_rename, if we find
> that the old inode is a directory, we *know* that the new
> one is a directory as well.
Ok, let's look  at the higher levels
We call vfs_rename (old_dir, old_dentry, new_dir, new_dentry), which
checks, wether old_dentry->d_inode is directory
If so it calls vfs_rename_dir, if not calls vfs_rename_other
In vfs_rename_dir checked if we can delete old_dentry (by means of may_delete), then if new_dentry->d_inode is present, check if we can delete it
But it is not checked, wether new_dentry->d_inode directory or not
I don't find this check in chain started from syscall and ends in ext2_rename
Correct me if I'm wrong.
Thanks in advance,
	Nikita

Re: Re[2]: Question about ext2

[Alexander Viro]

On Fri, 13 Jul 2001 malfet@gw.mipt.sw.ru wrote:

> I don't find this check in chain started from syscall and ends in ext2_rename
> Correct me if I'm wrong.

Take a look at may_delete(). Heck, it's even documented there -
 *  7. If we were asked to remove a directory and victim isn't one - ENOTDIR.
 *  8. If we were asked to remove a non-directory and victim isn't one - EISDIR.

Re: Question about ext2

[Alexander Viro]

On Fri, 13 Jul 2001, Andrew Morton wrote:

> I recently spent an hour decrypting this function.  Here is
> a commented version which may prove helpful. It is from a
> non-mainline branch of ext3, but it's much the same.

<raised brows> OK, here's an algorithm used in rename() - hopefully it
answers "dunno why" part.

	find directory entry of source or fail (-ENOENT)

	if moving directory
		find directory entry of ".." in object we move or fail (-EIO)

	if there is a victim
		if moving directory and victim is not empty - fail (-ENOTEMPTY)

		find directory entry of victim or fail (-ENOENT)

		grab a reference to old_inode and redirect that entry to it
		if victim is a directory
			drop reference to victim twice
		else
			drop reference to victim once
	else
		if moving directory
			check that we can grab an extra reference to new parent
			or fail with -EMLINK
		grab a reference to old inode
		create a link to old_inode (with new name)
		if link creation failed
			drop a reference we've just got and fail
		if it was a directory
			grab a reference to new parent

	/*
		Now we had created a new link to object we are moving
		and link to victim (if any) is destroyed.
	 */

	delete old link and drop the reference held by it.

	if moving directory
		redirect ".." to new parent and drop reference to old parent

Notice that we always grab a reference before creating a link and drop it
only after the link removal. All checks are done before any directory
modifications start - that way we can bail out if they fail.

The only really obscure part is dropping an extra reference if victim is
a directory - then we know that we are cannibalizing the last external
link to it and the only link that remains is victim's ".". We don't want
it to prevent victim's removal, so we drive i_nlink of victim to zero.

Re: Question about ext2

[Kai Henningsen]

viro@math.psu.edu (Alexander Viro)  wrote on 13.07.01 in <Pine.GSO.4.21.0107130623510.17323-100000@weyl.math.psu.edu>:

> The only really obscure part is dropping an extra reference if victim is
> a directory - then we know that we are cannibalizing the last external
> link to it and the only link that remains is victim's ".". We don't want
> it to prevent victim's removal, so we drive i_nlink of victim to zero.

Does this stuff work right with those cases which do linkcount=1 either  
because the fs doesn't have a link count, or because the real link count  
has grown too large?

MfG Kai

Re: Question about ext2

[Alexander Viro]

On 13 Jul 2001, Kai Henningsen wrote:

> viro@math.psu.edu (Alexander Viro)  wrote on 13.07.01 in <Pine.GSO.4.21.0107130623510.17323-100000@weyl.math.psu.edu>:
> 
> > The only really obscure part is dropping an extra reference if victim is
> > a directory - then we know that we are cannibalizing the last external
> > link to it and the only link that remains is victim's ".". We don't want
> > it to prevent victim's removal, so we drive i_nlink of victim to zero.
> 
> Does this stuff work right with those cases which do linkcount=1 either  
> because the fs doesn't have a link count, or because the real link count  
> has grown too large?

It doesn't. If fs doesn't have link count you are very likely to need
other ways to deal with rename() anyway (e.g. you are pretty likely to
have part of metadata stored in directory entry). If you are playing
with "set i_nlink to 1 if it's too large" (which works only for directories,
BTW) - change according to your encoding scheme for link count.

Re: Question about ext2

[Kai Henningsen]

viro@math.psu.edu (Alexander Viro)  wrote on 14.07.01 in <Pine.GSO.4.21.0107140151420.19749-100000@weyl.math.psu.edu>:

> On 13 Jul 2001, Kai Henningsen wrote:
>
> > viro@math.psu.edu (Alexander Viro)  wrote on 13.07.01 in
> > <Pine.GSO.4.21.0107130623510.17323-100000@weyl.math.psu.edu>:
> >
> > > The only really obscure part is dropping an extra reference if victim is
> > > a directory - then we know that we are cannibalizing the last external
> > > link to it and the only link that remains is victim's ".". We don't want
> > > it to prevent victim's removal, so we drive i_nlink of victim to zero.
> >
> > Does this stuff work right with those cases which do linkcount=1 either
> > because the fs doesn't have a link count, or because the real link count
> > has grown too large?
>
> It doesn't. If fs doesn't have link count you are very likely to need
> other ways to deal with rename() anyway (e.g. you are pretty likely to
> have part of metadata stored in directory entry). If you are playing
> with "set i_nlink to 1 if it's too large" (which works only for directories,
> BTW) - change according to your encoding scheme for link count.

You are, of course, aware that ext2 (or at least current patches to ext2,  
I'm not sure if this particular thing has gone in yet) does use that  
scheme.

MfG Kai