On Wed, Jan 16, 2013 at 03:22:52PM -0200, Lucas De Marchi wrote: > On Wed, Jan 16, 2013 at 12:02 PM, Dave Reisner <dreisner@xxxxxxxxxxxxx> wrote: > > Trust this value, instead of injecting our own, as this could be > > something as simple as ENOENT, rather than an alarming ENOMEM. > > --- > > libkmod/libkmod.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c > > index b3e1d6b..0d9cd80 100644 > > --- a/libkmod/libkmod.c > > +++ b/libkmod/libkmod.c > > @@ -816,7 +816,7 @@ KMOD_EXPORT int kmod_load_resources(struct kmod_ctx *ctx) > > > > fail: > > kmod_unload_resources(ctx); > > - return -ENOMEM; > > + return -errno; > > } > > Does this actually work? if index_mm_open() fails it may had passed > through munmap(), close() and free() before returning to > kmod_load_resources(). The first 2 can touch errno. Worse than that, > we also "goto fail" in index_mm_open() if the checks for magic or > version failed, in which case errno will not be set and we will > incorrectly return that this function succeeded. Sure, I tested this after all. With patch 2/2, but without this one: # rm /lib/modules/3.7.2-1-ARCH/modules.builtin* # modprobe --show-depends -S 3.7.2-1-ARCH # ./tools/modprobe --show-depends -S 3.7.2-1-ARCH ahci modprobe: ERROR: Failed to load module indicies: Cannot allocate memory And with the patch... # ./tools/modprobe --show-depends -S 3.7.2-1-ARCH ahci modprobe: ERROR: Failed to load module indicies: No such file or directory I hadn't delved too much into index_mm_open, but it sounds like the plumbing needs to be tightened up to keep a more reliable errno. > return -ENOMEM there maybe is not a good choice, but changing it to > may actually introduce bugs. Maybe -EINVAL or -1 would be a better > choice. Thoughts? I'd be against any proposition to hardcode an errno, since that's no better than what we have now. If we can get an accurate reason for the failure, we should definitely aim for that. d -- To unsubscribe from this list: send the line "unsubscribe linux-modules" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
