89 lines
3.0 KiB
Diff
89 lines
3.0 KiB
Diff
commit fa6b56faaa56c98203dcc3fbdda5eab3d91ec62d
|
|
Author: Josh Stone <jistone@redhat.com>
|
|
Date: Fri Jun 24 15:00:41 2011 -0700
|
|
|
|
rhbz716489: read instead of mmap to load modules
|
|
|
|
As staprun is preparing to load a kernel module, we first mmap the whole
|
|
module as MAP_PRIVATE. Then we proceed with our security checks,
|
|
including a trusted-signature validation on the mapped region, and if
|
|
all checks out, we'll call init_module() with that same mapped region.
|
|
|
|
However, MMAP(2) says of MAP_PRIVATE, "It is unspecified whether changes
|
|
made to the file after the mmap() call are visible in the mapped
|
|
region." From my testing, it appears that file changes do indeed show
|
|
up in our mapped memory. This means we have a TOCTOU race between
|
|
verifying the signature of that memory and then calling init_module().
|
|
|
|
By using read() instead of mmap(), we ensure that we have a fully
|
|
private copy of the module to verify and load, without fear of change.
|
|
|
|
diff --git a/runtime/staprun/staprun_funcs.c b/runtime/staprun/staprun_funcs.c
|
|
index 74eef9c..e0a5a46 100644
|
|
--- a/runtime/staprun/staprun_funcs.c
|
|
+++ b/runtime/staprun/staprun_funcs.c
|
|
@@ -49,7 +49,7 @@ int insert_module(
|
|
assert_permissions_func assert_permissions
|
|
) {
|
|
int i;
|
|
- long ret;
|
|
+ long ret, module_read;
|
|
void *module_file;
|
|
char *opts;
|
|
int saved_errno;
|
|
@@ -109,17 +109,39 @@ int insert_module(
|
|
return -1;
|
|
}
|
|
|
|
- /* mmap in the entire module. Work with the memory mapped data from this
|
|
- point on to avoid a TOCTOU race between path and signature checking
|
|
- below and module loading. */
|
|
- module_file = mmap(NULL, sbuf.st_size, PROT_READ, MAP_PRIVATE, module_fd, 0);
|
|
- if (module_file == MAP_FAILED) {
|
|
- _perr("Error mapping '%s'", module_realpath);
|
|
+ /* Allocate memory for the entire module. */
|
|
+ module_file = calloc(1, sbuf.st_size);
|
|
+ if (module_file == NULL) {
|
|
+ _perr("Error allocating memory to read '%s'", module_realpath);
|
|
close(module_fd);
|
|
free(opts);
|
|
return -1;
|
|
}
|
|
|
|
+ /* read in the entire module. Work with this copy of the data from this
|
|
+ point on to avoid a TOCTOU race between path and signature checking
|
|
+ below and module loading. */
|
|
+ module_read = 0;
|
|
+ while (module_read < sbuf.st_size) {
|
|
+ ret = read(module_fd, module_file + module_read,
|
|
+ sbuf.st_size - module_read);
|
|
+ if (ret > 0)
|
|
+ module_read += ret;
|
|
+ else if (ret == 0) {
|
|
+ _err("Unexpected EOF reading '%s'", module_realpath);
|
|
+ free(module_file);
|
|
+ close(module_fd);
|
|
+ free(opts);
|
|
+ return -1;
|
|
+ } else if (errno != EINTR) {
|
|
+ _perr("Error reading '%s'", module_realpath);
|
|
+ free(module_file);
|
|
+ close(module_fd);
|
|
+ free(opts);
|
|
+ return -1;
|
|
+ }
|
|
+ }
|
|
+
|
|
/* Check whether this module can be loaded by the current user.
|
|
* check_permissions will exit(-1) if permissions are insufficient*/
|
|
assert_permissions (module_realpath, module_fd, module_file, sbuf.st_size);
|
|
@@ -131,7 +153,7 @@ int insert_module(
|
|
|
|
/* Cleanup. */
|
|
free(opts);
|
|
- munmap(module_file, sbuf.st_size);
|
|
+ free(module_file);
|
|
close(module_fd);
|
|
|
|
if (ret != 0) {
|