locks: fix posix lock range overflow handling

In the 32-bit case fcntl assigns the 64-bit f_pos and i_size to a 32-bit off_t. The existing range checks also seem to depend on signed arithmetic wrapping when it overflows. In practice maybe that works, but we can be more careful. That also allows us to make a more reliable distinction between -EINVAL and -EOVERFLOW. Note that in the 32-bit case SEEK_CUR or SEEK_END might allow the caller to set a lock with starting point no longer representable as a 32-bit value. We could return -EOVERFLOW in such cases, but the locks code is capable of handling such ranges, so we choose to be lenient here. The only problem is that subsequent GETLK calls on such a lock will fail with EOVERFLOW. While we're here, do some cleanup including consolidating code for the flock and flock64 cases.

J. Bruce Fields committed 12 years ago

Commit ef12e721 parent 8c3cac52 files · +32 −71Browse files

Unified · Split

2 files

fs/locks.c +32 −68
include/uapi/asm-generic/fcntl.h +0 −3

fs/locks.c+32−68

@@ -344,48 +344,43 @@ static int assign_type(struct file_lock *fl, long type) · 343 unmodified lines
344	344	return 0;
345	345	}
346	346
347		−/* Verify a "struct flock" and copy it to a "struct file_lock" as a POSIX
348		− * style lock.
349		− */
350		−static int flock_to_posix_lock(struct file filp, struct file_lock fl,
351		− struct flock *l)
	347	+static int flock64_to_posix_lock(struct file filp, struct file_lock fl,
	348	+ struct flock64 *l)
352	349	{
353		− off_t start, end;
354		−
355	350	switch (l->l_whence) {
356	351	case SEEK_SET:
357		− start = 0;
	352	+ fl->fl_start = 0;
358	353	break;
359	354	case SEEK_CUR:
360		− start = filp->f_pos;
	355	+ fl->fl_start = filp->f_pos;
361	356	break;
362	357	case SEEK_END:
363		− start = i_size_read(file_inode(filp));
	358	+ fl->fl_start = i_size_read(file_inode(filp));
364	359	break;
365	360	default:
366	361	return -EINVAL;
367	362	}
	363	+ if (l->l_start > OFFSET_MAX - fl->fl_start)
	364	+ return -EOVERFLOW;
	365	+ fl->fl_start += l->l_start;
	366	+ if (fl->fl_start < 0)
	367	+ return -EINVAL;
368	368
369	369	/* POSIX-1996 leaves the case l->l_len < 0 undefined;
370	370	POSIX-2001 defines it. */
371		− start += l->l_start;
372		− if (start < 0)
373		− return -EINVAL;
374		− fl->fl_end = OFFSET_MAX;
375	371	if (l->l_len > 0) {
376		− end = start + l->l_len - 1;
377		− fl->fl_end = end;
	372	+ if (l->l_len - 1 > OFFSET_MAX - fl->fl_start)
	373	+ return -EOVERFLOW;
	374	+ fl->fl_end = fl->fl_start + l->l_len - 1;
	375	+
378	376	} else if (l->l_len < 0) {
379		− end = start - 1;
380		− fl->fl_end = end;
381		− start += l->l_len;
382		− if (start < 0)
	377	+ if (fl->fl_start + l->l_len < 0)
383	378	return -EINVAL;
384		− }
385		− fl->fl_start = start; /* we record the absolute position */
386		− if (fl->fl_end < fl->fl_start)
387		− return -EOVERFLOW;
388		−
	379	+ fl->fl_end = fl->fl_start - 1;
	380	+ fl->fl_start += l->l_len;
	381	+ } else
	382	+ fl->fl_end = OFFSET_MAX;
	383	+
389	384	fl->fl_owner = current->files;
390	385	fl->fl_pid = current->tgid;
391	386	fl->fl_file = filp;
@@ -396,52 +391,21 @@ static int flock_to_posix_lock(struct file filp, struct file_lock fl, · 4 unmodified lines
396	391	return assign_type(fl, l->l_type);
397	392	}
398	393
399		−#if BITS_PER_LONG == 32
400		−static int flock64_to_posix_lock(struct file filp, struct file_lock fl,
401		− struct flock64 *l)
	394	+/* Verify a "struct flock" and copy it to a "struct file_lock" as a POSIX
	395	+ * style lock.
	396	+ */
	397	+static int flock_to_posix_lock(struct file filp, struct file_lock fl,
	398	+ struct flock *l)
402	399	{
403		− loff_t start;
404		−
405		− switch (l->l_whence) {
406		− case SEEK_SET:
407		− start = 0;
408		− break;
409		− case SEEK_CUR:
410		− start = filp->f_pos;
411		− break;
412		− case SEEK_END:
413		− start = i_size_read(file_inode(filp));
414		− break;
415		− default:
416		− return -EINVAL;
417		− }
	400	+ struct flock64 ll = {
	401	+ .l_type = l->l_type,
	402	+ .l_whence = l->l_whence,
	403	+ .l_start = l->l_start,
	404	+ .l_len = l->l_len,
	405	+ };
418	406
419		− start += l->l_start;
420		− if (start < 0)
421		− return -EINVAL;
422		− fl->fl_end = OFFSET_MAX;
423		− if (l->l_len > 0) {
424		− fl->fl_end = start + l->l_len - 1;
425		− } else if (l->l_len < 0) {
426		− fl->fl_end = start - 1;
427		− start += l->l_len;
428		− if (start < 0)
429		− return -EINVAL;
430		− }
431		− fl->fl_start = start; /* we record the absolute position */
432		− if (fl->fl_end < fl->fl_start)
433		− return -EOVERFLOW;
434		−
435		− fl->fl_owner = current->files;
436		− fl->fl_pid = current->tgid;
437		− fl->fl_file = filp;
438		− fl->fl_flags = FL_POSIX;
439		− fl->fl_ops = NULL;
440		− fl->fl_lmops = NULL;
441		−
442		− return assign_type(fl, l->l_type);
	407	+ return flock64_to_posix_lock(filp, fl, &ll);
443	408	}
444		−#endif
445	409
446	410	/* default lease lock manager operations */
447	411	static void lease_break_callback(struct file_lock *fl)

include/uapi/asm-generic/fcntl.h+0−3

@@ -186,8 +186,6 @@ struct flock { · 185 unmodified lines
186	186	};
187	187	#endif
188	188
189		−#ifndef CONFIG_64BIT
190		−
191	189	#ifndef HAVE_ARCH_STRUCT_FLOCK64
192	190	#ifndef __ARCH_FLOCK64_PAD
193	191	#define __ARCH_FLOCK64_PAD
@@ -202,6 +200,5 @@ struct flock64 { · 8 unmodified lines
202	200	__ARCH_FLOCK64_PAD
203	201	};
204	202	#endif
205		−#endif /* !CONFIG_64BIT */
206	203
207	204	#endif /* _ASM_GENERIC_FCNTL_H */