Hi Robert, On Wed, Jun 20, 2012 at 9:47 AM, Robert Milasan <[email protected]> wrote: > On Wed, 20 Jun 2012 09:15:33 -0300 > Lucas De Marchi <[email protected]> wrote: > >> On Wed, Jun 20, 2012 at 8:56 AM, Robert Milasan <[email protected]> >> wrote: >> > On Wed, 20 Jun 2012 08:34:59 -0300 >> > Lucas De Marchi <[email protected]> wrote: >> > >> >> Hi Robert, >> >> >> >> please CC [email protected] so others can benefit from >> >> the responses. I'm doing so now. >> >> >> >> On Wed, Jun 20, 2012 at 8:21 AM, Robert Milasan <[email protected]> >> >> wrote: >> >> > Hello, I'm trying now to build kmod 9 on 64 and 32 bits >> >> > opensuse. On 64bits everything is fine, but on 32bits it fails >> >> > in init_modules.c: 150:assert(mkdir_p(buf, 0755) >= 0); >> >> > >> >> > If I drop assert, all works. Do you have an idea why it fails? >> >> >> >> Does "make check" work for you after removing this assert? Some of >> >> the tests that rely on being able to create dirs on testsuite's >> >> rootfs should fail if you didn't create the dirs. >> >> >> >> On your 32 bits machine, is your testsuite/rootfs dir (or any of >> >> the dirs below) read-only? >> >> >> >> Lucas De Marchi >> > >> > If I drop assert(mkdir_p...) then all works. rootfs and the below >> > dirs all have the rights "0755" so all good, but this issue doesn't >> > happen on 64bits which is a bit strange for me. Maybe there is >> > something wrong in mkdir.c -> mkdir_p function. >> >> Maybe... gotta check this on a 32 bits machine. >> >> > >> > Anyway I've asked around and some people said that using assert on >> > "mkdir_p" is kind of a bad idea, but I'm not a developer so I >> > wouldn't know. >> >> Yeah, except that this code path runs only on *testsuite*, i.e. it's >> part of the tests we make to check it's right, but it's not part of >> libkmod, modprobe or depmod, etc that runs on production. >> >> I put the assert because there are tests that depend on being able to >> create dirs: among others, the "install command loop" test. If dir is >> not created, it will fail nonetheless, but instead of saying it's >> because of a failed dir creation it will create a fork bomb (yeah, >> it's a loop of fork + exec). I preferred failing the test earlier than >> having to killall modprobe later or wait for kernel's oom killer. >> >> >> Lucas De Marchi > > OK, now I know why it fails errno -17 which means the dir already > exists. > > test-init: testsuite/init_module.c:155: create_sysfs_files: Assertion > `err >= 0' failed. err: -17 > > This is a small patch for that:
Thanks for this.
The bug is a bit deeper: neither on mkdir_p or the filesystem
permissions. The testsuite includes a 64 bits module that we need to
fake its insertion (both in 64 and 32 bits machine) - and that means
picking the module name at the correct memory offset. This is why it
fails only for 32 bits machine - it was using the "32-bits offset" to
parse a module for 64 bits and the end result was: modname = "".
The patch below (the same as attached) fixed the issue for me on 32
bits. Let me know if it works for you
Lucas De Marchi
----------8<-----------
diff --git a/testsuite/init_module.c b/testsuite/init_module.c
index 814998a..8089636 100644
--- a/testsuite/init_module.c
+++ b/testsuite/init_module.c
@@ -16,6 +16,7 @@
*/
#include <assert.h>
+#include <elf.h>
#include <errno.h>
#include <dirent.h>
#include <fcntl.h>
@@ -206,6 +207,12 @@ static inline bool module_is_inkernel(const char *modname)
return ret;
}
+static uint8_t elf_identify(void *mem)
+{
+ uint8_t *p = mem;
+ return p[EI_CLASS];
+}
+
TS_EXPORT long init_module(void *mem, unsigned long len, const char *args);
/*
@@ -225,6 +232,8 @@ long init_module(void *mem, unsigned long len,
const char *args)
const void *buf;
uint64_t bufsize;
int err;
+ uint8_t class;
+ off_t offset;
init_retcodes();
@@ -236,14 +245,18 @@ long init_module(void *mem, unsigned long len,
const char *args)
&bufsize);
kmod_elf_unref(elf);
- /*
- * We couldn't find the module's name inside the ELF file. Just exit
- * as if it was successful
- */
+ /* We couldn't parse the ELF file. Just exit as if it was successful */
if (err < 0)
return 0;
- modname = (char *)buf + offsetof(struct module, name);
+ /* We need to open both 32 and 64 bits module - hack! */
+ class = elf_identify(mem);
+ if (class == ELFCLASS64)
+ offset = MODULE_NAME_OFFSET_64;
+ else
+ offset = MODULE_NAME_OFFSET_32;
+
+ modname = (char *)buf + offset;
mod = find_module(modules, modname);
if (mod != NULL) {
errno = mod->errcode;
diff --git a/testsuite/stripped-module.h b/testsuite/stripped-module.h
index 9f97dae..19862f3 100644
--- a/testsuite/stripped-module.h
+++ b/testsuite/stripped-module.h
@@ -13,6 +13,7 @@ struct list_head {
};
#define MODULE_NAME_LEN (64 - sizeof(unsigned long))
+
struct module
{
enum module_state state;
@@ -24,4 +25,8 @@ struct module
char name[MODULE_NAME_LEN];
};
+/* padding */
+#define MODULE_NAME_OFFSET_64 4 + 4 + 2 * 8
+#define MODULE_NAME_OFFSET_32 4 + 2 * 4
+
#endif
fix-32bits.diff
Description: Binary data
