LockFile() should error with ERROR_INVALID_PARAMETER on overflow
authorJonathan Pryor <jonpryor@vt.edu>
Wed, 9 Jan 2013 16:59:09 +0000 (11:59 -0500)
committerJonathan Pryor <jonpryor@vt.edu>
Wed, 9 Jan 2013 16:59:09 +0000 (11:59 -0500)
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=9411

What was happening was that this code:

var file = new FileStream (...);
file.Lock (0, long.MaxValue);

was failing on Android (a non-large file platform):

System.IO.IOException: Lock violation on path...

The fact that it errored is correct. The problem is that the error
isn't particularly helpful and is misleading; WHY is there a lock
violation? There isn't a lock violation; the parameters are
invalid.

Why are the parameters invalid? Because `long.MaxValue` was used as
the `length`, which resulted in an `off_t` value of -1 [0] for the
file lock length, which fcntl(2) (rightfully!) errored on.

Update _wapi_lock_file_region()/LockFile() to check for negative
values and set ERROR_INVALID_PARAMETER when invalid values are used.
This will cause an "IOException: Invalid parameter" message, instead
of an "IOException: Lock violation on path" message, which is
considerably more useful.

Related: we noticed that the generation of `offset` and `length` in
LockFile()'s !HAVE_LARGE_FILE_SUPPORT region are incorrect, e.g. this
is broken:

length = length_low;

Assume a `file.Lock(0, 0x100000000L)` invocation. After splitting,
offset_high will be 0x1 and offset_low will be 0x0, so length is 0x0,
which is not semantically correct (it's not at all what the developer
requested!), yet it would result in a successful fcntl(2) call
(because length_high is ignored).

Add an explicit check for length_high and offset_high when
HAVE_LARGE_FILE_SUPPORT isn't set and error out if they're > 0, which
will catch this case.

[0]: fcntl(2) uses off_t for file offset and file lock length,
and off_t is a signed type:

http://www.unix.org/version2/whatsnew/lfs.html#2.2.2.6
> The types blkcnt_t and off_t are defined as extended signed integral types.

mono/io-layer/locking.c

index 57b18ca662e936243e78c14908fb6cf87d26cc4a..eec2549513dba01f0eb3a00093c7bf45afdad883 100644 (file)
@@ -34,6 +34,11 @@ _wapi_lock_file_region (int fd, off_t offset, off_t length)
        struct flock lock_data;
        int ret;
 
+       if (offset < 0 || length < 0) {
+               SetLastError (ERROR_INVALID_PARAMETER);
+               return(FALSE);
+       }
+
        lock_data.l_type = F_WRLCK;
        lock_data.l_whence = SEEK_SET;
        lock_data.l_start = offset;
@@ -146,6 +151,10 @@ LockFile (gpointer handle, guint32 offset_low, guint32 offset_high,
 
        DEBUG ("%s: Locking handle %p, offset %lld, length %lld", __func__, handle, offset, length);
 #else
+       if (offset_high > 0 || length_high > 0) {
+               SetLastError (ERROR_INVALID_PARAMETER);
+               return (FALSE);
+       }
        offset = offset_low;
        length = length_low;