Re: Busybox modprobe hitch with kernel 5.10
On Sun, Jan 3, 2021 at 2:59 AM Qu Wenruo wrote: > >> If you really want to make finit_modules() to support compressed module, > >> purpose a new system call other than changing something which already > >> has a clear and simple spec. > > > > I decided to just wait when inevitably someone adds that support. > > I don't really see why doing the decompression inside the kernel is a > good idea. > Thus the "inevitably" part doesn't make sense to me. Wanna bet? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Busybox modprobe hitch with kernel 5.10
On Sat, Jan 2, 2021 at 2:48 AM Qu Wenruo wrote: > On 2021/1/1 下午7:05, Denys Vlasenko wrote: > > I propose this simple fix: > > > > if (not_a_good_ELF_image(image_ptr)) { > > + if (!not_GZIP_signature(image_ptr) > > +|| !not_BZIP2_signature(image_ptr) > > +|| !not_XZ_signature(image_ptr)) > > printk(KERN_ERR "Module has invalid ELF header\n"); > > } > > > > There are several problems. > > Firstly, what if a valid compressed file is passed in but without a > valid ELF header? > Now we fall back to old behavior where user has no clue at all. > > Secondly, you are going to add new decompression related headers and > extra refactor, which is never a small fix. > Especially considering how complex the compressed signature can be. gz, bz2 and xz signatures are trivial. > Finally, what if the decompression module is compressed? > Just consider this: if xz support is compiled as a module, and that > modules is also xz compressed? > > If you really want to make finit_modules() to support compressed module, > purpose a new system call other than changing something which already > has a clear and simple spec. I decided to just wait when inevitably someone adds that support. I'm not planning to modify busybox modprobe to avoid generating the dmesg message. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Busybox modprobe hitch with kernel 5.10
On Thu, Dec 31, 2020 at 2:04 AM Qu Wenruo wrote: > On 2020/12/31 上午8:50, Denys Vlasenko wrote: > > On Thu, Dec 31, 2020 at 1:45 AM Qu Wenruo wrote: > >> If kernel is really going to detect compressed module, the decompression > >> should happen before we check the ELF header, thus the error message > >> should still be there. > > > > I was talking from the userspace perspective: is there a reasonable way > > to be more careful for userspace? No, unless userspace assumes > > that finit_module() will *never* be extended to handle .ko.gz modules. > > It's the spec, if finit_module() really supports compressed kernel one > day, it breaks the spec and only then it should be considered as a > regression (ironically) If we go down legalese road, the spec does not say "if you supply something which is not an ELF image, kernel will emit a scary message to the log". I propose this simple fix: if (not_a_good_ELF_image(image_ptr)) { + if (!not_GZIP_signature(image_ptr) +|| !not_BZIP2_signature(image_ptr) +|| !not_XZ_signature(image_ptr)) printk(KERN_ERR "Module has invalid ELF header\n"); } ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Busybox modprobe hitch with kernel 5.10
On Thu, Dec 31, 2020 at 1:45 AM Qu Wenruo wrote: > >> Ah, I see what's happening. modprobe normally tries finit_module() > >> first, and falls back to init_module() for compressed modules, which > >> is the case here. We added new error messages as part of an effort to > >> demystify module loader errors, since sometimes we don't always know > >> why a module fails to load (See upstream kernel commit: 14721add58ef). > >> > >> Since loading compressed modules will always produce this message from > >> finit_module(), I would be fine with removing the message for now as a > >> workaround. Perhaps the long-term solution would be to teach > >> finit_module() to work with compressed modules. Wenruo, since you were > >> the commit author, would you be fine with dropping the pr_err() message? > > Not exactly. > > In fact, the error message matches what the > init_module(2)/finit_module(2) says: > > init_module() loads an ELF image into kernel space, performs any > necessary symbol relocations ... > > We're not supposed to load compressed data from the very first place, > thus the ELF header check is valid, and doing its work. > > And consider such case, where a kernel module is really corrupted, > without the message, how would the end user really know what's going wrong? > > Not every end user would be able to add extra pr_*() calls into the > kernel and find out the problem. > > Really, it's the user space module loader not doing it proper checking. User space module loader has no idea whether kernel can accept a gzipped or xz-ed module via finit_module(). > > I can add a condition to not try finit_module() on compressed modules, > > however, I anticipate needing to revert it when kernel learns how to load > > compressed modules as well. > > > > If kernel is really going to detect compressed module, the decompression > should happen before we check the ELF header, thus the error message > should still be there. I was talking from the userspace perspective: is there a reasonable way to be more careful for userspace? No, unless userspace assumes that finit_module() will *never* be extended to handle .ko.gz modules. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Busybox modprobe hitch with kernel 5.10
On Wed, Dec 30, 2020 at 12:28 PM Jessica Yu wrote: > +++ Lauri Kasanen [30/12/20 10:08 +0200]: > >On Tue, 29 Dec 2020 22:05:21 +0100 > >Denys Vlasenko wrote: > > > >> On Mon, Dec 28, 2020 at 6:15 PM Lauri Kasanen wrote: > >> > Hi, > >> > > >> > On Busybox 1.31.1, modprobing any module for the 5.10 kernel prints a > >> > nasty message: > >> > Module has invalid ELF header > >> > > >> > but the module loads successfully after that. So looks like bb tries > >> > two paths, the first fails, second succeeds. > >> > >> What does strace show? > > > >openat(AT_FDCWD, "kernel/drivers/char/ppdev.ko.gz", > >O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3 > >SYS_350(0x3, 0x80b37d5, 0, 0xb7d794b7, 0x3) = -1 ENOEXEC (Exec format error) > >close(3)= 0 > >openat(AT_FDCWD, "kernel/drivers/char/ppdev.ko.gz", O_RDONLY|O_LARGEFILE) = 3 > >read(3, "\37\213", 2) = 2 > >read(3, > >"\10\0\0\0\0\0\2\3\214V\5p\333h\26\226l'\345\332s\353\366\312\350\36S\2713\245\364 > 16384) = 4363 > >read(3, "", 12021) = 0 > >read(3, "", 16384) = 0 > >close(3)= 0 > >init_module(0x921b9c0, 10100, "") = 0 > > > >Full log attached. Repros on both x86 and x86_64, and the modules are gzip > >compressed. > > > >- Lauri > > Ah, I see what's happening. modprobe normally tries finit_module() > first, and falls back to init_module() for compressed modules, which > is the case here. We added new error messages as part of an effort to > demystify module loader errors, since sometimes we don't always know > why a module fails to load (See upstream kernel commit: 14721add58ef). > > Since loading compressed modules will always produce this message from > finit_module(), I would be fine with removing the message for now as a > workaround. Perhaps the long-term solution would be to teach > finit_module() to work with compressed modules. Wenruo, since you were > the commit author, would you be fine with dropping the pr_err() message? I can add a condition to not try finit_module() on compressed modules, however, I anticipate needing to revert it when kernel learns how to load compressed modules as well. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Busybox modprobe hitch with kernel 5.10
On Tue, 29 Dec 2020 22:05:21 +0100 Denys Vlasenko wrote: > On Mon, Dec 28, 2020 at 6:15 PM Lauri Kasanen wrote: > > Hi, > > > > On Busybox 1.31.1, modprobing any module for the 5.10 kernel prints a > > nasty message: > > Module has invalid ELF header > > > > but the module loads successfully after that. So looks like bb tries > > two paths, the first fails, second succeeds. > > What does strace show? openat(AT_FDCWD, "kernel/drivers/char/ppdev.ko.gz", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3 SYS_350(0x3, 0x80b37d5, 0, 0xb7d794b7, 0x3) = -1 ENOEXEC (Exec format error) close(3)= 0 openat(AT_FDCWD, "kernel/drivers/char/ppdev.ko.gz", O_RDONLY|O_LARGEFILE) = 3 read(3, "\37\213", 2) = 2 read(3, "\10\0\0\0\0\0\2\3\214V\5p\333h\26\226l'\345\332s\353\366\312\350\36S\2713\245\364 strace.log.gz Description: Binary data ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Busybox modprobe hitch with kernel 5.10
On Mon, Dec 28, 2020 at 6:15 PM Lauri Kasanen wrote: > Hi, > > On Busybox 1.31.1, modprobing any module for the 5.10 kernel prints a > nasty message: > Module has invalid ELF header > > but the module loads successfully after that. So looks like bb tries > two paths, the first fails, second succeeds. What does strace show? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox