[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v3 0/1] kern/dl: Add module version check
Re: [RFC PATCH v3 0/1] kern/dl: Add module version check
Fri, 23 Dec 2022 15:18:22 +0800
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1
I think there are some misunderstandings.
This feature is implemented in kern/dl.c in core.img, which is UNDER
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
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.
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.
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.
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
think you are expert in this area. If that patch finally got merged
(e.g. before 2.12 release window), you life will be definitely easier
because you are free from shipping another bunch of core.img. However,
since I'm not a expert of this area, please point out if I said
On 2022/12/23 02:16, Pete Batard wrote:
I'm sorry but that's NO_GO for me.
This is a major step back from v2, where the check was never applicable
for BIOS and I believe I explained why a module check that can be
enforced for BIOS is going to create a major headache for end-users.
It seems pretty obvious that, if distro maintainers start to use
--enable-modver-check=enforce for UEFI they are also going to use the
same option for BIOS (because why should they use a different
compilation options if it applies for both?), and you *ARE* going to
leave Windows users stranded through them not trying to boot GRUB based
media because they will be forced to use DD to create a workable media
(even though, and this is the major point here, in the very vast
majority of cases there is not issue with mixing versions if you're on
the same major/minor base for GRUB), and consider that, because they
cannot see its content, or can only see the ESP, their media was not
properly created and is not worth trying to boot.
This is a *VERY REAL* problem, for which I have provided many examples
(see ), and could provide even more, which is why utilities like
Rufus try to steer away from using DD mode where possible.
However, if people can use --enable-modver-check=enforce for BIOS, then,
under the aim of solving one minor issue (that from looking the Google
links for "452: out of range pointer" seem to be a problem that is
mostly encountered in UEFI environments rather than BIOS, or at least is
not a BIOS-only issue, and therefore something that will be sorted out
for the majority of folks experiencing it *even* if
--enable-modver-check cannot apply for BIOS), you will be adding to an
already existing a major one, and you are going to leave people, and
especially Windows users who are trying to switch to Linux, stranded.
In short, GRUB would be doing a major disservice to its end users if it
is to allow module version checks to be enforceable for BIOS, and,
considering how there is little benefit in enforcing this in an
environment that cannot be secured in the first place, it's hard to see
a justification for what is essentially a *MAJOR* design regression from
the v2 of this patch. Or at the very least, a better approach is to
introduce it for UEFI only, then, once that has been integrated with
UEFI for a while, see if BIOS users are actually reporting issues due to
module version mismatch, and reconvene on whether we really want to
allow this check to also apply to BIOS.
Thus, in my view, the proper approach for the sake of the end users is
leave default BIOS boot behaviour as it currently stands, with no
warning and no possibility of failing the boot process on version
mismatch (because, and I am well placed to know, both from experience as
well as the feedback I get from Rufus users, that in the very vast
majority of cases, as long as you use modules and core.img based on the
same major+minor, there is no issue, and producing a warning that hints
that there may be is only going to be detrimental to the end user).
And, once again, I have to ask this mailing list to please try not to
consider issues only from a Linux/UNIX based maintainer's perspective,
but also consider them from the end-users who are actually going to be
creating GRUB based media on Windows, where using ISOHybrid as DD is far
from the panacea it's supposed to be.
Finally, if you are interested in seeing how end users actually struggle
with boot media and boot media creation, I *strongly* invite you to
spend some time on reddit, askubuntu, superuser and other end-user
places, where you should be able to validate that, as I have pointed
out, Windows end users really do struggle with ISOHybrdids that are
written in DD mode, which will hopefully help you reach the conclusion
that, if this v3 proposal gets integrated, it will cause more harm for
BIOS users than it will benefit them on account that distro maintainers
are bound to start using --enable-modver-check=enforce everywhere, if it
can apply to both BIOS and UEFI.
On 2022.12.22 16:38, Zhang Boyang wrote:
This is the V3 of my patch.
V2 is at:
V1 is at:
[ TD;LR ]
1) The check is always enforced when GRUB is locked down, i.e. modules
will be refused to load if they have mismatched version
2) If built with "--disable-modver-check", modules can always be
loaded even if they have mismatched version, and no message will be
3) If built with "--enable-modver-check=audit", modules can always be
loaded even if they have mismatched version, but if mismatch is found,
a warning message will be displayed. This is the default.
4) If built with "--enable-modver-check=enforce", the behavior is same
[ Why this patch is useful, even for BIOS boot ]
Because it helps people diagnose broken (or improper) GRUB installations.
For example, if you google "452: out of range pointer", you will got a
lot of results in 2022. I think the most of them are related to
mismatched modules. However, these problem are often not properly
diagnosed because they disappear magically, e.g. update whole system
(which triggers grub reinstall). There are several people even suspect
there are problems with their hard disk / BIOS. However, the root
cause is 052e6068be62 ("mm: When adding a region, merge with region
after as well as before") changed the layout of `struct
grub_mm_region`, which is both used in main program and
"relocator.mod", so the module reads the wrong field and crashes GRUB.
Please the commit did nothing wrong because there is no API/ABI
compatibility guarantees in GRUB.
If there are warning messages about mismatched modules, user will
easily notice there are problems with their GRUB installation.
[ Why not enforce this check to prevent crashes ]
As Glenn & Pete said, most mismatched modules isn't harmful. At most
times, GRUB with mismatched modules can boot Linux happily, even if
these modules come from another Linux distribution. This enables user
to fix his/her GRUB installation without using a boot/rescue disk,
because the user can boot the existing Linux using the existing (but
improperly installed) GRUB.
[ Why warning can be disabled ]
Some tools like Rufus relies on mismatched modules. Some advanced
users also doesn't like redundant warnings for their existing
However, it's highly unrecommended to disable this warning.
[ Why this patch is a prerequisite for external signed module support ]
Consider this scenario:
1) GRUB 2.XX is free of vulnerabilities
2) GRUB 2.YY is also free of vulnerabilities
3) So GRUB 2.XX shares same SBAT numbers with GRUB 2.YY, therefore
SBAT can't help in version check
4) If there is no version check, it's possible to load GRUB 2.YY
modules into GRUB 2.XX (and vice versa)
5) However, due to some changes in API or ABI, although unlikely,
there is possibility that there are vulnerabilities when using GRUB
2.YY modules with GRUB 2.XX (and vice versa)
6) So we must enforce version check to prevent this from happening
However, because version string is only consisted of PACKAGE_VERSION,
it must be unique for one given vendor (signer). For example, version
string need to be different for Debian 10 and Debian 11 even they both
use GRUB 2.06, and no two build in Debian 10 (or Debian 11) have same