On Wed, Nov 20, 2013 at 5:39 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
> On Wed, Nov 20, 2013 at 5:01 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>> From: Miklos Szeredi <mszeredi@xxxxxxx>
>>
>> If flags contain RENAME_EXCHANGE then exchange source and destination files.
>> There's no restriction on the type of the files; e.g. a directory can be
>> exchanged with a symlink.
>
> What happens if both RENAME_EXCHANGE and RENAME_NOREPLACE are set?
It fails with EINVAL, since it's a nonsensical combination.
RENAME_EXCHANGE requires the target to exist, RENAME_NOREPLACE
requires the target to _not_ exist.
Thanks,
Miklos
>
> --Andy
>
>>
>> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxx>
>> ---
>> fs/dcache.c | 46 +++++++++++++++++----
>> fs/namei.c | 107 +++++++++++++++++++++++++++++++++---------------
>> include/linux/dcache.h | 1 +
>> include/uapi/linux/fs.h | 1 +
>> security/security.c | 16 ++++++++
>> 5 files changed, 131 insertions(+), 40 deletions(-)
>>
>> diff --git a/fs/dcache.c b/fs/dcache.c
>> index 0a38ef8d7f00..ea2fca1a2ec9 100644
>> --- a/fs/dcache.c
>> +++ b/fs/dcache.c
>> @@ -2527,12 +2527,14 @@ static void switch_names(struct dentry *dentry, struct dentry *target)
>> dentry->d_name.name = dentry->d_iname;
>> } else {
>> /*
>> - * Both are internal. Just copy target to dentry
>> + * Both are internal.
>> */
>> - memcpy(dentry->d_iname, target->d_name.name,
>> - target->d_name.len + 1);
>> - dentry->d_name.len = target->d_name.len;
>> - return;
>> + unsigned int i;
>> + BUILD_BUG_ON(!IS_ALIGNED(DNAME_INLINE_LEN, sizeof(long)));
>> + for (i = 0; i < DNAME_INLINE_LEN / sizeof(long); i++) {
>> + swap(((long *) &dentry->d_iname)[i],
>> + ((long *) &target->d_iname)[i]);
>> + }
>> }
>> }
>> swap(dentry->d_name.len, target->d_name.len);
>> @@ -2589,13 +2591,15 @@ static void dentry_unlock_parents_for_move(struct dentry *dentry,
>> * __d_move - move a dentry
>> * @dentry: entry to move
>> * @target: new dentry
>> + * @exchange: exchange the two dentries
>> *
>> * Update the dcache to reflect the move of a file name. Negative
>> * dcache entries should not be moved in this way. Caller must hold
>> * rename_lock, the i_mutex of the source and target directories,
>> * and the sb->s_vfs_rename_mutex if they differ. See lock_rename().
>> */
>> -static void __d_move(struct dentry * dentry, struct dentry * target)
>> +static void __d_move(struct dentry *dentry, struct dentry *target,
>> + bool exchange)
>> {
>> if (!dentry->d_inode)
>> printk(KERN_WARNING "VFS: moving negative dcache entry\n");
>> @@ -2619,6 +2623,10 @@ static void __d_move(struct dentry * dentry, struct dentry * target)
>>
>> /* Unhash the target: dput() will then get rid of it */
>> __d_drop(target);
>> + if (exchange) {
>> + __d_rehash(target,
>> + d_hash(dentry->d_parent, dentry->d_name.hash));
>> + }
>>
>> list_del(&dentry->d_u.d_child);
>> list_del(&target->d_u.d_child);
>> @@ -2645,6 +2653,8 @@ static void __d_move(struct dentry * dentry, struct dentry * target)
>> write_seqcount_end(&dentry->d_seq);
>>
>> dentry_unlock_parents_for_move(dentry, target);
>> + if (exchange)
>> + fsnotify_d_move(target);
>> spin_unlock(&target->d_lock);
>> fsnotify_d_move(dentry);
>> spin_unlock(&dentry->d_lock);
>> @@ -2662,11 +2672,31 @@ static void __d_move(struct dentry * dentry, struct dentry * target)
>> void d_move(struct dentry *dentry, struct dentry *target)
>> {
>> write_seqlock(&rename_lock);
>> - __d_move(dentry, target);
>> + __d_move(dentry, target, false);
>> write_sequnlock(&rename_lock);
>> }
>> EXPORT_SYMBOL(d_move);
>>
>> +/*
>> + * d_exchange - exchange two dentries
>> + * @dentry1: first dentry
>> + * @dentry2: second dentry
>> + */
>> +void d_exchange(struct dentry *dentry1, struct dentry *dentry2)
>> +{
>> + write_seqlock(&rename_lock);
>> +
>> + WARN_ON(!dentry1->d_inode);
>> + WARN_ON(!dentry2->d_inode);
>> + WARN_ON(IS_ROOT(dentry1));
>> + WARN_ON(IS_ROOT(dentry2));
>> +
>> + __d_move(dentry1, dentry2, true);
>> +
>> + write_sequnlock(&rename_lock);
>> +}
>> +
>> +
>> /**
>> * d_ancestor - search for an ancestor
>> * @p1: ancestor dentry
>> @@ -2714,7 +2744,7 @@ static struct dentry *__d_unalias(struct inode *inode,
>> m2 = &alias->d_parent->d_inode->i_mutex;
>> out_unalias:
>> if (likely(!d_mountpoint(alias))) {
>> - __d_move(alias, dentry);
>> + __d_move(alias, dentry, false);
>> ret = alias;
>> }
>> out_err:
>> diff --git a/fs/namei.c b/fs/namei.c
>> index 5b41d4bfecd1..c23621255df0 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -4025,6 +4025,8 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>> const unsigned char *old_name;
>> struct inode *source = old_dentry->d_inode;
>> struct inode *target = new_dentry->d_inode;
>> + bool new_is_dir = false;
>> + unsigned max_links = new_dir->i_sb->s_max_links;
>>
>> if (source == target)
>> return 0;
>> @@ -4033,10 +4035,16 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>> if (error)
>> return error;
>>
>> - if (!target)
>> + if (!target) {
>> error = may_create(new_dir, new_dentry);
>> - else
>> - error = may_delete(new_dir, new_dentry, is_dir);
>> + } else {
>> + new_is_dir = d_is_dir(new_dentry);
>> +
>> + if (!(flags & RENAME_EXCHANGE))
>> + error = may_delete(new_dir, new_dentry, is_dir);
>> + else
>> + error = may_delete(new_dir, new_dentry, new_is_dir);
>> + }
>> if (error)
>> return error;
>>
>> @@ -4047,10 +4055,17 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>> * If we are going to change the parent - check write permissions,
>> * we'll need to flip '..'.
>> */
>> - if (is_dir && new_dir != old_dir) {
>> - error = inode_permission(source, MAY_WRITE);
>> - if (error)
>> - return error;
>> + if (new_dir != old_dir) {
>> + if (is_dir) {
>> + error = inode_permission(source, MAY_WRITE);
>> + if (error)
>> + return error;
>> + }
>> + if ((flags & RENAME_EXCHANGE) && new_is_dir) {
>> + error = inode_permission(target, MAY_WRITE);
>> + if (error)
>> + return error;
>> + }
>> }
>>
>> error = security_inode_rename(old_dir, old_dentry, new_dir, new_dentry,
>> @@ -4060,25 +4075,24 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>>
>> old_name = fsnotify_oldname_init(old_dentry->d_name.name);
>> dget(new_dentry);
>> - if (!is_dir)
>> - lock_two_nondirectories(source, target);
>> - else if (target)
>> - mutex_lock(&target->i_mutex);
>> + if (!(flags & RENAME_EXCHANGE)) {
>> + if (!is_dir)
>> + lock_two_nondirectories(source, target);
>> + else if (target)
>> + mutex_lock(&target->i_mutex);
>> + }
>>
>> error = -EBUSY;
>> if (d_mountpoint(old_dentry) || d_mountpoint(new_dentry))
>> goto out;
>>
>> - if (is_dir) {
>> - unsigned max_links = new_dir->i_sb->s_max_links;
>> -
>> + if (max_links && new_dir != old_dir) {
>> error = -EMLINK;
>> - if (max_links && !target && new_dir != old_dir &&
>> - new_dir->i_nlink >= max_links)
>> + if (is_dir && !new_is_dir && new_dir->i_nlink >= max_links)
>> + goto out;
>> + if ((flags & RENAME_EXCHANGE) && !is_dir && new_is_dir &&
>> + old_dir->i_nlink > max_links)
>> goto out;
>> -
>> - if (target)
>> - shrink_dcache_parent(new_dentry);
>> } else {
>> error = try_break_deleg(source, delegated_inode);
>> if (error)
>> @@ -4089,27 +4103,40 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>> goto out;
>> }
>> }
>> + if (is_dir && !(flags & RENAME_EXCHANGE) && target)
>> + shrink_dcache_parent(new_dentry);
>> error = old_dir->i_op->rename(old_dir, old_dentry, new_dir, new_dentry,
>> flags);
>> if (error)
>> goto out;
>>
>> - if (target) {
>> + if (!(flags & RENAME_EXCHANGE) && target) {
>> if (is_dir)
>> target->i_flags |= S_DEAD;
>> dont_mount(new_dentry);
>> }
>> - if (!(old_dir->i_sb->s_type->fs_flags & FS_RENAME_DOES_D_MOVE))
>> - d_move(old_dentry, new_dentry);
>> + if (!(old_dir->i_sb->s_type->fs_flags & FS_RENAME_DOES_D_MOVE)) {
>> + if (!(flags & RENAME_EXCHANGE))
>> + d_move(old_dentry, new_dentry);
>> + else
>> + d_exchange(old_dentry, new_dentry);
>> + }
>> out:
>> - if (!is_dir)
>> - unlock_two_nondirectories(source, target);
>> - else if (target)
>> - mutex_unlock(&target->i_mutex);
>> + if (!(flags & RENAME_EXCHANGE)) {
>> + if (!is_dir)
>> + unlock_two_nondirectories(source, target);
>> + else if (target)
>> + mutex_unlock(&target->i_mutex);
>> + }
>> dput(new_dentry);
>> - if (!error)
>> + if (!error) {
>> fsnotify_move(old_dir, new_dir, old_name, is_dir,
>> - target, old_dentry);
>> + !(flags & RENAME_EXCHANGE) ? target : NULL, old_dentry);
>> + if (flags & RENAME_EXCHANGE) {
>> + fsnotify_move(new_dir, old_dir, old_dentry->d_name.name,
>> + new_is_dir, NULL, new_dentry);
>> + }
>> + }
>> fsnotify_oldname_free(old_name);
>>
>> return error;
>> @@ -4129,9 +4156,12 @@ SYSCALL_DEFINE5(renameat2, int, olddfd, const char __user *, oldname,
>> bool should_retry = false;
>> int error;
>>
>> - if (flags & ~RENAME_NOREPLACE)
>> + if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE))
>> return -EOPNOTSUPP;
>>
>> + if ((flags & RENAME_NOREPLACE) && (flags & RENAME_EXCHANGE))
>> + return -EINVAL;
>> +
>> retry:
>> from = user_path_parent(olddfd, oldname, &oldnd, lookup_flags);
>> if (IS_ERR(from)) {
>> @@ -4166,7 +4196,8 @@ retry:
>>
>> oldnd.flags &= ~LOOKUP_PARENT;
>> newnd.flags &= ~LOOKUP_PARENT;
>> - newnd.flags |= LOOKUP_RENAME_TARGET;
>> + if (!(flags & RENAME_EXCHANGE))
>> + newnd.flags |= LOOKUP_RENAME_TARGET;
>>
>> retry_deleg:
>> trap = lock_rename(new_dir, old_dir);
>> @@ -4186,12 +4217,23 @@ retry_deleg:
>> error = -EEXIST;
>> if ((flags & RENAME_NOREPLACE) && d_is_positive(new_dentry))
>> goto exit5;
>> + if (flags & RENAME_EXCHANGE) {
>> + error = -ENOENT;
>> + if (!new_dentry->d_inode)
>> + goto exit5;
>> +
>> + if (!d_is_dir(new_dentry)) {
>> + error = -ENOTDIR;
>> + if (newnd.last.name[newnd.last.len])
>> + goto exit5;
>> + }
>> + }
>> /* unless the source is a directory trailing slashes give -ENOTDIR */
>> if (!d_is_dir(old_dentry)) {
>> error = -ENOTDIR;
>> if (oldnd.last.name[oldnd.last.len])
>> goto exit5;
>> - if (newnd.last.name[newnd.last.len])
>> + if (!(flags & RENAME_EXCHANGE) && newnd.last.name[newnd.last.len])
>> goto exit5;
>> }
>> /* source should not be ancestor of target */
>> @@ -4199,7 +4241,8 @@ retry_deleg:
>> if (old_dentry == trap)
>> goto exit5;
>> /* target should not be an ancestor of source */
>> - error = -ENOTEMPTY;
>> + if (!(flags & RENAME_EXCHANGE))
>> + error = -ENOTEMPTY;
>> if (new_dentry == trap)
>> goto exit5;
>>
>> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
>> index 901616910e0a..87e14e698f89 100644
>> --- a/include/linux/dcache.h
>> +++ b/include/linux/dcache.h
>> @@ -306,6 +306,7 @@ extern void dentry_update_name_case(struct dentry *, struct qstr *);
>>
>> /* used for rename() and baskets */
>> extern void d_move(struct dentry *, struct dentry *);
>> +extern void d_exchange(struct dentry *, struct dentry *);
>> extern struct dentry *d_ancestor(struct dentry *, struct dentry *);
>>
>> /* appendix may either be NULL or be used for transname suffixes */
>> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
>> index 9250f4dd7d96..ca1a11bb4443 100644
>> --- a/include/uapi/linux/fs.h
>> +++ b/include/uapi/linux/fs.h
>> @@ -36,6 +36,7 @@
>> #define SEEK_MAX SEEK_HOLE
>>
>> #define RENAME_NOREPLACE (1 << 0) /* Don't overwrite target */
>> +#define RENAME_EXCHANGE (1 << 1) /* Exchange source and dest */
>>
>> struct fstrim_range {
>> __u64 start;
>> diff --git a/security/security.c b/security/security.c
>> index d00ae28dea76..0ef72cd2255f 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -439,6 +439,14 @@ int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
>> if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
>> (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
>> return 0;
>> +
>> + if (flags & RENAME_EXCHANGE) {
>> + int err = security_ops->path_rename(new_dir, new_dentry,
>> + old_dir, old_dentry);
>> + if (err)
>> + return err;
>> + }
>> +
>> return security_ops->path_rename(old_dir, old_dentry, new_dir,
>> new_dentry);
>> }
>> @@ -531,6 +539,14 @@ int security_inode_rename(struct inode *old_dir, struct dentry *old_dentry,
>> if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
>> (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
>> return 0;
>> +
>> + if (flags & RENAME_EXCHANGE) {
>> + int err = security_ops->inode_rename(new_dir, new_dentry,
>> + old_dir, old_dentry);
>> + if (err)
>> + return err;
>> + }
>> +
>> return security_ops->inode_rename(old_dir, old_dentry,
>> new_dir, new_dentry);
>> }
>> --
>> 1.8.1.4
>>
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
