On Wed, Dec 11, 2013 at 09:37:24AM -0500, J. Bruce Fields wrote:
> Well, it'd be weird if I didn't screw up something somewhere.
Yes, a classic: I forgot the breaks after each switch case.
Here's a version that at least doesn't return -EINVAL on every lock
attempt.
--b.
commit 70b7b9a442d06a71306736eb01cedba5dd6d86cf
Author: J. Bruce Fields <bfields@xxxxxxxxxx>
Date: Tue Dec 10 18:14:28 2013 -0500
locks: fix posix lock range overflow handling
In the 32-bit case fcntl is converting signed 64-bit to signed 32-bit
quantities in a couple places, with probably incorrect results.
So instead let's return -EOVERFLOW as described in SUSv3, whenever "the
smallest or, if l_len is non-zero, the largest offset of any byte in the
requested segment cannot be represented correctly in an object of type
off_t."
While we're here, do some cleanup including consolidating code for the
flock and flock64 cases.
Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
diff --git a/fs/locks.c b/fs/locks.c
index 92a0f0a..b70486a 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -344,48 +344,44 @@ static int assign_type(struct file_lock *fl, long type)
return 0;
}
-/* Verify a "struct flock" and copy it to a "struct file_lock" as a POSIX
- * style lock.
- */
-static int flock_to_posix_lock(struct file *filp, struct file_lock *fl,
- struct flock *l)
+static int flock_to_posix_lock_common(struct file *filp, struct file_lock *fl,
+ struct flock64 *l, loff_t offset_max)
{
- off_t start, end;
+ loff_t start;
switch (l->l_whence) {
case SEEK_SET:
- start = 0;
+ fl->fl_start = 0;
break;
case SEEK_CUR:
- start = filp->f_pos;
+ fl->fl_start = filp->f_pos;
break;
case SEEK_END:
- start = i_size_read(file_inode(filp));
+ fl->fl_start = i_size_read(file_inode(filp));
break;
default:
return -EINVAL;
}
+ if (l->l_start < 0)
+ return -EINVAL;
+ if (l->l_start > offset_max - fl->fl_start)
+ return -EOVERFLOW;
+ fl->fl_start += l->l_start;
+ if (l->l_len > offset_max - fl->fl_start)
+ return -EOVERFLOW;
+ if (fl->fl_start + l->l_len < 0)
+ return -EINVAL;
/* POSIX-1996 leaves the case l->l_len < 0 undefined;
POSIX-2001 defines it. */
- start += l->l_start;
- if (start < 0)
- return -EINVAL;
- fl->fl_end = OFFSET_MAX;
- if (l->l_len > 0) {
- end = start + l->l_len - 1;
- fl->fl_end = end;
- } else if (l->l_len < 0) {
- end = start - 1;
- fl->fl_end = end;
- start += l->l_len;
- if (start < 0)
- return -EINVAL;
- }
- fl->fl_start = start; /* we record the absolute position */
- if (fl->fl_end < fl->fl_start)
- return -EOVERFLOW;
-
+ if (l->l_len > 0)
+ fl->fl_end = fl->fl_start + l->l_len - 1;
+ else if (l->l_len < 0) {
+ fl->fl_end = start - 1;
+ fl->fl_start += l->l_len;
+ } else
+ fl->fl_end = OFFSET_MAX;
+
fl->fl_owner = current->files;
fl->fl_pid = current->tgid;
fl->fl_file = filp;
@@ -396,50 +392,27 @@ static int flock_to_posix_lock(struct file *filp, struct file_lock *fl,
return assign_type(fl, l->l_type);
}
+/* Verify a "struct flock" and copy it to a "struct file_lock" as a POSIX
+ * style lock.
+ */
+static int flock_to_posix_lock(struct file *filp, struct file_lock *fl,
+ struct flock *l)
+{
+ struct flock64 ll = {
+ .l_type = l->l_type,
+ .l_whence = l->l_whence,
+ .l_start = l->l_start,
+ .l_len = l->l_len,
+ };
+
+ return flock_to_posix_lock_common(filp, fl, &ll, OFFT_OFFSET_MAX);
+}
+
#if BITS_PER_LONG == 32
static int flock64_to_posix_lock(struct file *filp, struct file_lock *fl,
struct flock64 *l)
{
- loff_t start;
-
- switch (l->l_whence) {
- case SEEK_SET:
- start = 0;
- break;
- case SEEK_CUR:
- start = filp->f_pos;
- break;
- case SEEK_END:
- start = i_size_read(file_inode(filp));
- break;
- default:
- return -EINVAL;
- }
-
- start += l->l_start;
- if (start < 0)
- return -EINVAL;
- fl->fl_end = OFFSET_MAX;
- if (l->l_len > 0) {
- fl->fl_end = start + l->l_len - 1;
- } else if (l->l_len < 0) {
- fl->fl_end = start - 1;
- start += l->l_len;
- if (start < 0)
- return -EINVAL;
- }
- fl->fl_start = start; /* we record the absolute position */
- if (fl->fl_end < fl->fl_start)
- return -EOVERFLOW;
-
- fl->fl_owner = current->files;
- fl->fl_pid = current->tgid;
- fl->fl_file = filp;
- fl->fl_flags = FL_POSIX;
- fl->fl_ops = NULL;
- fl->fl_lmops = NULL;
-
- return assign_type(fl, l->l_type);
+ return flock_to_posix_lock_common(filp, fl, l, OFFSET_MAX);
}
#endif
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 95e46c8..36025f7 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -186,8 +186,6 @@ struct flock {
};
#endif
-#ifndef CONFIG_64BIT
-
#ifndef HAVE_ARCH_STRUCT_FLOCK64
#ifndef __ARCH_FLOCK64_PAD
#define __ARCH_FLOCK64_PAD
@@ -202,6 +200,5 @@ struct flock64 {
__ARCH_FLOCK64_PAD
};
#endif
-#endif /* !CONFIG_64BIT */
#endif /* _ASM_GENERIC_FCNTL_H */
--
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
