date:20221223

Re: [RFC PATCH v2 1/1] kern/dl: Add module version check

2022-12-23 Thread Zhang Boyang


Hi,

On 2022/12/22 14:31, Glenn Washburn wrote:

On Wed, 21 Dec 2022 20:11:57 +0800
Zhang Boyang  wrote:

+  grub_printf_ (N_("warning: module %.63s has incorrect version:
%.63s != %s\n"),
+   mod->name, modver, PACKAGE_VERSION);


I don't quite like this, but I could live with it. I mostly don't like
it because I don't want to get spammed with a lot of these warning
messages. But maybe this is the best of the bad set of options.



I sent a V3 patch at 
https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00280.html , in 
which can disable this warning at build time, however I personally don't 
like it.


I think preloading modules is a good solution for you. e.g. run `insmod 
foobar` before switching $root/$prefix. This also eliminate the 
possibility of GRUB crashing because of mismatched modules. For a 
comprehensive list of modules, please see $GRUB_MODULES in 
https://salsa.debian.org/grub-team/grub/-/blob/master/debian/build-efi-images


If you think this solution is good to you, I want to drop V3 and submit 
a improved V2 as V4, because it's cleaner.



One thing to note is I believe that this will take GRUB out
of graphical mode and put it into text mode. This doesn't affect me
(right now) though.



I tested on i386-pc and x86_64-efi and there is no such behavior.


Another thing is that, for the use case of loading a grub.cfg from a
different GRUB installation, this message from version mismatched
modules loaded in the sourcing of the script will be displayed for a
fraction of a second until the menu from the sourced grub.cfg is
displayed. So it may not be helpful for the user.



What about adding a 100ms sleep when the warning is displayed? By the 
way, this can also be workaround by preloading modules.



This isn't likely an issue for loopback.cfg because usually the module
loading is done implicitly and all the commands are generally inside
the menu entries. In this case, printing to the screen seems like a
good idea because directly after the module loading the commands will
run, which might crash GRUB. So if the crash hangs the machine the user
will see this message and have a clue.



Best Regards,
Zhang Boyang


Glenn



___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFC PATCH v3 0/1] kern/dl: Add module version check

2022-12-23 Thread Pete Batard via Grub-devel


On 2022.12.23 07:18, Zhang Boyang wrote:
This feature is implemented in kern/dl.c in core.img, which is UNDER 
YOUR CONTROL.


Let me analyze the worst case from your perspective:

1) Every distro is enforcing version check, even in BIOS mode.

2) Because this check is in kern/dl.c, there is no such code in any 
module (.mod) files. However, this modules are little fatter because 
they have version information (.modver section), which is harmless.


3) You can still build your own copy of core.img with this check 
disabled (or just revert this patch completely), then ship that core.img 
with Rufus.


4) Since there is no check in your core.img, it will load any module 
happily, regardless whether it has mismatched version information.


Therefore, there is no problem at all. The only thing you need to do is 
build your core.img with this patch reverted in the future. Then things 
will "work" as usual.


I need to properly test this, but I think that should work, yes.

I should have looked a bit more closely at your proposal, because I 
agree that it should address my concerns, though I do have a small worry 
about the possibility of transitive checks in the future (i.e. modules 
that perform their own loading of submodules for whatever reason, with a 
version check being introduced there).


I'm not planning to try to hold your patch on that, but I sure hope that 
transitive version checks never become a thing.

Let me analyze why I want to add version check to BIOS mode:

1) Because you can always remove this patch from your build for Rufus, 
there is no harm in having this patch in vanilla GRUB.


As long as the version check remains localised inside core.img, yes.

2) This patch can provide benefits to BIOS users, because it can 
diagnose improper installations. For example, user runs `grub-install 
/dev/sdb` to update GRUB but machine boot from `/dev/sda`.


So there are benefits without any harm, why not have it in BIOS mode?

By the way, I highly recommend you to test latest git GRUB (as a preview 
of upcoming 2.12 release) with Rufus and it will almost certainly break 
your existing Rufus build. e.g. use Rufus (with GRUB2.06) to process 
ISOs (with GRUB2.12); and the reverse, use Rufus (with GRUB2.12) to 
process ISOs (with GRUB2.06). You probably need to ship another bunch of 
core.img with Rufus.


That's already been the case for previous releases, e.g. trying to use 
core.img from 2.0(x) with 2.0(x-1) is usually asking for trouble, and I 
do have a mechanism in place to address just that, where users don't 
even need to update Rufus as we can always download a new 'core.img' 
server from our repository. This is also the mechanism that helps us 
address the rare case where distro maintainers have applied breaking 
pacthes onto a dot release that affect core.img.


So, once distros start using GRUB 2.12 (because I need to be able to 
validate the core.img I build against an actual image), I'll have a 
core.img for GRUB 2.12 that users can automatically use in Rufus.


As I pointed out before, the main issue is that even when using the same 
base version, and because GRUB releases are far and in-between, distro 
maintainers cherry pick patches to apply on top of the dot release, and 
(usually, which is the proper thing to do) suffix their version id, 
which of course, would make a version check fail.


But we're still working with the same dot release base in Rufus always, 
which is why the core.img replacement (barring a few exceptions) always 
works.


Also, I'd also like to advice you to create a patchset/proposal, which 
make it easy to convert the bootcode in ISO to the bootcode in USB.


I considered it a long time ago, but the main issue is that it would 
only help with future GRUB based images, and not the many many current 
ones I needed to support.


Thus, all it would accomplish then, is force me to have two different 
methods of solving a problem, where one can most certainly do, and hope 
that, in 10 years time (at best, because you bet some people will still 
want to use 10-years old ISOs then, especially if they are booting old 
BIOS-based computers) I *might* be able to remove one of them.


I hope you can appreciate how this approach makes very little sense then 
when one method can accomplish all we need.


Plus, trying to reuse the files from El-Torito would be a major 
challenge, as, even if we were to somehow force include part_msdos, 
biosdisk, fat and ntfs always (which are completely unneeded for optical 
boot) and recover them, we'd still need to alter the prefix directory, 
and that resides in a part that is usually compressed.


So, the only viable alternative, would be to force optical media 
creators to generate a core.img that can work for USB boot (which means 
silent invocation of grub-mkimage behind the scenes), and have that 
somehow find its way on the resulting ISO media.


All in all, these are fairly heavy alterations to the GRUB optical boot 
generation proce

Re: [RFC PATCH v3 0/1] kern/dl: Add module version check

2022-12-23 Thread Zhang Boyang

Hi,

On 2022/12/23 21:07, Pete Batard wrote:

On 2022.12.23 07:18, Zhang Boyang wrote:
This feature is implemented in kern/dl.c in core.img, which is UNDER
YOUR CONTROL.

Let me analyze the worst case from your perspective:

1) Every distro is enforcing version check, even in BIOS mode.

2) Because this check is in kern/dl.c, there is no such code in any
module (.mod) files. However, this modules are little fatter because
they have version information (.modver section), which is harmless.

3) You can still build your own copy of core.img with this check
disabled (or just revert this patch completely), then ship that
core.img with Rufus.

4) Since there is no check in your core.img, it will load any module
happily, regardless whether it has mismatched version information.

Therefore, there is no problem at all. The only thing you need to do
is build your core.img with this patch reverted in the future. Then
things will "work" as usual.

I need to properly test this, but I think that should work, yes.

I think the result will be OK. If so, it's good news for both of us.

I should have looked a bit more closely at your proposal, because I
agree that it should address my concerns, though I do have a small worry
about the possibility of transitive checks in the future (i.e. modules
that perform their own loading of submodules for whatever reason, with a
version check being introduced there).

I'm not planning to try to hold your patch on that, but I sure hope that
transitive version checks never become a thing.

By the way, I'm planning to drop V3 and reuse V2 as the base of V4 (if
Glenn's concerns is solved). So there will be no "enforce" for BIOS at
all in V4, thus the "small worry" is eliminated. However, I will
probably add sleep(100ms) to warning message. But don't worry, you can
patch your own build like this

- || grub_dl_check_version (mod, e)
+ || (1||grub_dl_check_version (mod, e))

to completely disable the check.

Let me analyze why I want to add version check to BIOS mode:

1) Because you can always remove this patch from your build for Rufus,
there is no harm in having this patch in vanilla GRUB.

As long as the version check remains localised inside core.img, yes.

2) This patch can provide benefits to BIOS users, because it can
diagnose improper installations. For example, user runs `grub-install
/dev/sdb` to update GRUB but machine boot from `/dev/sda`.

So there are benefits without any harm, why not have it in BIOS mode?

By the way, I highly recommend you to test latest git GRUB (as a
preview of upcoming 2.12 release) with Rufus and it will almost
certainly break your existing Rufus build. e.g. use Rufus (with
GRUB2.06) to process ISOs (with GRUB2.12); and the reverse, use Rufus
(with GRUB2.12) to process ISOs (with GRUB2.06). You probably need to
ship another bunch of core.img with Rufus.

OK, That's great news.

That's already been the case for previous releases, e.g. trying to use
core.img from 2.0(x) with 2.0(x-1) is usually asking for trouble, and I
do have a mechanism in place to address just that, where users don't
even need to update Rufus as we can always download a new 'core.img'
server from our repository. This is also the mechanism that helps us
address the rare case where distro maintainers have applied breaking
pacthes onto a dot release that affect core.img.

So, once distros start using GRUB 2.12 (because I need to be able to
validate the core.img I build against an actual image), I'll have a
core.img for GRUB 2.12 that users can automatically use in Rufus.

As I pointed out before, the main issue is that even when using the same
base version, and because GRUB releases are far and in-between, distro
maintainers cherry pick patches to apply on top of the dot release, and
(usually, which is the proper thing to do) suffix their version id,
which of course, would make a version check fail.

But we're still working with the same dot release base in Rufus always,
which is why the core.img replacement (barring a few exceptions) always
works.

FYI, Ubuntu, Debian, and some other distros are likely to update their
GRUB version (dot release) during life cycle, mostly because of secure
boot related fixes. For example,
https://tracker.debian.org/news/1352218/accepted-grub2-206-3deb10u1-source-into-oldstable-proposed-updates-oldstable-new-oldstable-proposed-updates/

Personally speaking, I guess the number of exceptions might be
underestimated. Because most ISOs were using ISOLINUX instead of GRUB2
in the past, and there seems a trend of replacing ISOLINUX with GRUB2.
For example, Ubuntu 20.04 uses ISOLINUX and Ubuntu 20.10 uses GRUB2, so
there is only 2 years of experience of using mismatched GRUB2 for
Ubuntu. So I guess problem might arise when more and more distros switch
to GRUB2. However, this is only a guess, and we both hope it's not true.

Also, I'd also like to advice you to create a patchset/propo

Re: [RFC PATCH v3 0/1] kern/dl: Add module version check

2022-12-23 Thread Zhang Boyang


On 2022/12/24 00:44, Zhang Boyang wrote:

-  || grub_dl_check_version (mod, e)
+  || (1||grub_dl_check_version (mod, e))


Oh, there is a small error, it should be:

-  || grub_dl_check_version (mod, e)
+  || (0 && grub_dl_check_version (mod, e))

Best Regards,
Zhang Boyang

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

22 matches

Mail list logo