linux-kernel - Re: [BUGFIX][PATCH 3/3] configfs: Fix failing symlink() making rmdir() fail

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: <20080617221528.GA29091@mail.oracle.com>
Date:	Tue, 17 Jun 2008 15:15:28 -0700
From:	Joel Becker <Joel.Becker@...cle.com>
To:	Louis Rilling <louis.rilling@...labs.com>
Cc:	linux-kernel@...r.kernel.org, ocfs2-devel@....oracle.com
Subject: Re: [BUGFIX][PATCH 3/3] configfs: Fix failing symlink() making
	rmdir() fail

On Tue, Jun 17, 2008 at 07:37:23PM +0200, Louis Rilling wrote:
> For the parent's rmdir() case, we can use the same solution as with mkdir() vs
> rmdir(). For the target's rmdir() case, we cannot, since we do not and cannot
> lock the target's inode while in symlink(). Fortunately, once create_link()
> terminates, no further operation can fail in symlink(). So we first reorder the
> operations in create_link() to attach the new symlink to its target in last
> place, and second handle symlink creation failure the same way as a new item
> creation failure.

	Oh, no, ugh.  We don't want to create vfs objects first and ask
questions later.  Otherwise we wouldn't need ATTACHING - we'd just
create the symlink, then check dropping.
	If you have ATTACHING set, the rmdir cannot continue - you can
check dropping at that time.  That is, you keep the DROPPING check where
it is - if it is already set, you know that rmdir() is going to complete
successfully.  You can bail before even calling configfs_create_link().
If, however, it isn't set, your ATTACHING protects you from rmdir
throughout.
 
	sl = kmalloc(sizeof(struct configfs_symlink), GFP_KERNEL);
 	if (sl) {
 		sl->sl_target = config_item_get(item);
 		spin_lock(&configfs_dirent_lock);
		if (target_sd->s_type & CONFIGFS_USET_DROPPING) {
			spin_unlock(&configfs_dirent_lock);
			config_item_put(item);
			kfree(sl);
			return -ENOENT;
			/*
		 	* Force rmdir() of parent_item to wait until we know
		 	* if we succeed.
		 	*/
			parent_sd->s_type |= CONFIGFS_USET_ATTACHING;
		}
		list_add(&sl->sl_list, &target_sd->s_links);
 		spin_unlock(&configfs_dirent_lock);
 		ret = configfs_create_link(sl, parent_item->ci_dentry,
 					   dentry);
		spin_lock(&configfs_dirent_lock);
		parent_sd->s_type &= ~CONFIGFS_USET_ATTACHING;
		if (ret) {
			list_del_init(&sl->sl_list);
 			spin_unlock(&configfs_dirent_lock);
 			config_item_put(item);
 			kfree(sl);
 		} else
			spin_unlock(&configfs_dirent_lock);
 	}
 
 	return ret;

-- 

"When arrows don't penetrate, see...
 Cupid grabs the pistol."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@...cle.com
Phone: (650) 506-8127
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/