2013/5/10 Jeff Layton jlayton@poochiereds.net:
On Fri, 26 Apr 2013 16:04:16 +0400 Pavel Shilovsky piastry@etersoft.ru wrote:
Introduce new LOCK_DELETE flock flag that is suggested to be used internally only to map O_DENYDELETE open flag:
!O_DENYDELETE -> LOCK_DELETE | LOCK_MAND.
Signed-off-by: Pavel Shilovsky piastry@etersoft.ru
fs/locks.c | 53 +++++++++++++++++++++++++++++++++------- fs/namei.c | 3 +++ include/linux/fs.h | 6 +++++ include/uapi/asm-generic/fcntl.h | 1 + 4 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c index dbc5557..1cc68a9 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -269,7 +269,7 @@ EXPORT_SYMBOL(locks_copy_lock);
static inline int flock_translate_cmd(int cmd) { if (cmd & LOCK_MAND)
return cmd & (LOCK_MAND | LOCK_RW);
return cmd & (LOCK_MAND | LOCK_RW | LOCK_DELETE); switch (cmd) { case LOCK_SH: return F_RDLCK;@@ -614,6 +614,8 @@ deny_flags_to_cmd(unsigned int flags) cmd |= LOCK_READ; if (!(flags & O_DENYWRITE)) cmd |= LOCK_WRITE;
if (!(flags & O_DENYDELETE))cmd |= LOCK_DELETE; return cmd;} @@ -836,6 +838,31 @@ out: return error; }
+int +sharelock_may_delete(struct dentry *dentry) +{
struct file_lock **before;int rc = 0;if (!IS_SHARELOCK(dentry->d_inode))return rc;lock_flocks();for_each_lock(dentry->d_inode, before) {struct file_lock *fl = *before;if (IS_POSIX(fl))break;if (IS_LEASE(fl))continue;if (fl->fl_type & LOCK_DELETE)continue;rc = 1;break;}unlock_flocks();return rc;+}
/*
- Determine if a file is allowed to be opened with specified access and share
- modes. Lock the file and return 0 if checks passed, otherwise return
@@ -850,10 +877,6 @@ sharelock_lock_file(struct file *filp) if (!IS_SHARELOCK(filp->f_path.dentry->d_inode)) return error;
/* Disable O_DENYDELETE support for now */if (filp->f_flags & O_DENYDELETE)return -EINVAL;error = flock_make_lock(filp, &lock, deny_flags_to_cmd(filp->f_flags)); if (error) return error;@@ -1717,6 +1740,12 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd) if (!f.file) goto out;
/* LOCK_DELETE is defined to be translated from O_DENYDELETE only */if (cmd & LOCK_DELETE) {error = -EINVAL;goto out;}can_sleep = !(cmd & LOCK_NB); cmd &= ~LOCK_NB; unlock = (cmd == LOCK_UN);@@ -2261,10 +2290,16 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, seq_printf(f, "UNKNOWN UNKNOWN "); } if (fl->fl_type & LOCK_MAND) {
seq_printf(f, "%s ",(fl->fl_type & LOCK_READ)? (fl->fl_type & LOCK_WRITE) ? "RW " : "READ ": (fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE ");
if (fl->fl_type & LOCK_DELETE)seq_printf(f, "%s ",(fl->fl_type & LOCK_READ) ?(fl->fl_type & LOCK_WRITE) ? "RWDEL" : "RDDEL" :(fl->fl_type & LOCK_WRITE) ? "WRDEL" : "DEL ");elseseq_printf(f, "%s ",(fl->fl_type & LOCK_READ) ?(fl->fl_type & LOCK_WRITE) ? "RW " : "READ " :(fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE "); } else { seq_printf(f, "%s ", (lease_breaking(fl))diff --git a/fs/namei.c b/fs/namei.c index dd236fe..a404f7d 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2220,6 +2220,7 @@ static inline int check_sticky(struct inode *dir, struct inode *inode)
- We can't remove a root or mountpoint.
- We don't allow removal of NFS sillyrenamed files; it's handled by
nfs_async_unlink().
*/
- We can't do it if victim is locked by O_DENYDELETE sharelock.
static int may_delete(struct inode *dir,struct dentry *victim,int isdir) { @@ -2250,6 +2251,8 @@ static int may_delete(struct inode *dir,struct dentry *victim,int isdir) return -ENOENT; if (victim->d_flags & DCACHE_NFSFS_RENAMED) return -EBUSY;
if (sharelock_may_delete(victim))return -ESHAREDENIED;Is there a potential race here?
You're holding the parent's i_mutex when setting a lock on this file, but you're not holding it when you test for it here. So it seems possible you could end up granting a O_DENYDELETE open on a file that is in the process of being deleted from the namespace.
may_delete function is called from vfs_unlnk, vfs_rename and vfs_rmdir and in all those places the caller is holding parent's i_mutex. It seems that the locking order is correct: we hold parent's i_mutex when we set and test sharelocks.
return 0;}
diff --git a/include/linux/fs.h b/include/linux/fs.h index 24066d2..afd56b1 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1006,6 +1006,7 @@ extern int lock_may_read(struct inode *, loff_t start, unsigned long count); extern int lock_may_write(struct inode *, loff_t start, unsigned long count); extern void locks_delete_block(struct file_lock *waiter); extern int sharelock_lock_file(struct file *); +extern int sharelock_may_delete(struct dentry *); extern void lock_flocks(void); extern void unlock_flocks(void); #else /* !CONFIG_FILE_LOCKING */ @@ -1159,6 +1160,11 @@ static inline int sharelock_lock_file(struct file *filp) return 0; }
+static inline int sharelock_may_delete(struct dentry *dentry) +{
return 0;+}
static inline void lock_flocks(void) { } diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h index 5ac0d49..a3e6349 100644 --- a/include/uapi/asm-generic/fcntl.h +++ b/include/uapi/asm-generic/fcntl.h @@ -167,6 +167,7 @@ struct f_owner_ex { blocking */ #define LOCK_UN 8 /* remove lock */
+#define LOCK_DELETE 16 /* which allows to delete a file */ #define LOCK_MAND 32 /* This is a mandatory flock ... */ #define LOCK_READ 64 /* which allows concurrent read operations */ #define LOCK_WRITE 128 /* which allows concurrent write operations */
-- Jeff Layton jlayton@poochiereds.net -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-- Best regards, Pavel Shilovsky.