[PATCH] filemap ffffffffull bogosity

From: Hugh Dickins
Date: Mon Oct 20 2003 - 09:13:37 EST


Hi Marcelo,

Here's the 2.4.23-pre7 version of a fix we made in 2.5.67 at beginning
of April, following discussion with Alan on LKML. Last week I verified
that current code indeed behaves very wrongly with "ulimit -f 5000000"
on ia64 (writes write more than the specified count).

When handling rlimit != RLIM_INFINITY, precheck_file_write tests file
position against 0xFFFFFFFFULL, and casts it to a u32. This code is
carried forward from 2.4.4, and 2.4-ac tree used to have an apparently
obvious fix to one part of it (should set count to 0 not to a negative).
But when you think it through, it all turns out to be bogus.

On a 32-bit architecture: limit is a 32-bit unsigned long, we've
already handled *pos < 0 and *pos >= limit, so *pos here has no way
of being > 0xFFFFFFFFULL, and thus casting it to u32 won't truncate it.
And on a 64-bit architecture: limit is a 64-bit unsigned long, but this
code is disallowing file position beyond the 32 bits; or if there's some
userspace compatibility issue, with limit having to fit into 32 bits,
the 32-bit architecture argument applies and they're still irrelevant.

So just remove the 0xFFFFFFFFULL test; and in place of the u32, cast to
typeof(limit) so it's right even if rlimits get wider. And there's no
way we'd want to send SIGXFSZ below the limit: remove send_sig comment.

There's a similarly suspicious u32 cast a little further down, when
checking MAX_NON_LFS. Given its definition, that does no harm on any
arch: but it's better changed to unsigned long, the type of MAX_NON_LFS.

Hugh

--- 2.4.23-pre7/mm/filemap.c 2003-10-10 21:32:07.000000000 +0100
+++ linux/mm/filemap.c 2003-10-10 21:33:37.000000000 +0100
@@ -3031,9 +3031,8 @@
send_sig(SIGXFSZ, current, 0);
goto out;
}
- if (pos > 0xFFFFFFFFULL || *count > limit - (u32)pos) {
- /* send_sig(SIGXFSZ, current, 0); */
- *count = limit - (u32)pos;
+ if (*count > limit - (typeof(limit))pos) {
+ *count = limit - (typeof(limit))pos;
}
}

@@ -3045,9 +3044,8 @@
send_sig(SIGXFSZ, current, 0);
goto out;
}
- if (*count > MAX_NON_LFS - (u32)pos) {
- /* send_sig(SIGXFSZ, current, 0); */
- *count = MAX_NON_LFS - (u32)pos;
+ if (*count > MAX_NON_LFS - (unsigned long)pos) {
+ *count = MAX_NON_LFS - (unsigned long)pos;
}
}


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/