> Having an undocumented (or silently installed, therefore unexpected) > dependency is undesirable (especially for a firmware), no question about that.
Sorry, I still get the impression that there is a fundamental misunderstanding about what Git submodules are here. It's *just* source code! It's source code that comes cryptographically guaranteed from the main coreboot Git repository, exactly like all the normal coreboot source code. Nothing is "silently installed" on your system here or anything. It's just a way to organize some source code as a separate unit that can manually be resynced with changes from another upstream. It's not really different from what we did for LZ4 compression code in cbfstool, except that it makes organization and regular updates easier than copying in the whole thing directly. And I don't know what you mean by "undocumented", we have more documentation on the vboot stuff than most parts of coreboot (see Documentation/security/vboot... also, there's Documentation/getting_started/submodules.md). > Can you guarantee that a silently installed submodule's repo won't get hacked > and replaced with malicious code for example? We have seen that happening > with other repos already (github, sourceforge and other webhosts too). The > fewer dependency you have, the less are the chances for a vulnerability or > sechole, simple as that. Yes, because the Git SHA of the submodule HEAD is stored in the main coreboot repo. I explained this in my last mail already. This is not node.js, if the submodule changes externally that will not affect coreboot's use of it until a coreboot developer intentionally pushes an update to the coreboot repository itself. > Julius also mentioned elsewhere that the same issues that can sneak into > vboot can hit us anywhere else. That's generally true but, IMHO, highly > unlikely. When we work on central parts of the build process (e.g. cbfstool) > and review patches there, we take care of portability and keep it to standard > C, FWIW. (I know people like to use packed structs, but that's not too funky, > I guess.) No, I don't think most people submitting to cbfstool are somehow applying some magic unwritten portability standard that vboot is lacking to be honest. And if they do, and you can tell me what it is, we are happy to apply the same standard to vboot going forward. Like I said earlier, I am perfectly happy to align vboot standards and practices to coreboot to solve this kind of interworking friction, the problem is just that *we don't have any standards*! I mean, it's not like we somehow blindly added __attribute__((fallback)) to vboot thinking it was standard C89, we were fully aware that this was a somewhat modern feature in GCC and clang, and everyone we talked to back then (I think this was on Gerrit somewhere, not sure how to find it right now, sorry) agreed that this was fine and that aligning HOSTCC requirements to crossgcc made sense. Since apparently enough people here feel that it's a big problem all of a sudden (even though I think it landed close to a year ago) I'm going to take it out again now and then hopefully we can move on. And if people are concerned about getting hit by problems like this in the future (both in submodules and normal coreboot code) it would be great if someone could just put something in Documentation/ to define what we're intending to support, so we don't need to have discussions again about what some but clearly not all people might maybe be looking out for in reviews if they happen to spot it. I don't think "just write it fully portable" is a reasonable goal since clearly we need __attributes__ like ((packed)), and there are other common extensions without which writing C code is just a huge pain in general (e.g. compound statements for double-evaluation safe MIN()/MAX()), so I'd prefer if we could just pick a minimum GCC and clang version number. And then maybe one day we could even get Jenkins to test that. (Also, while we're on the subject of submodules causing pain, Nico... whenever I want to build test older Intel generations I have to first figure out again which of them don't rely on libgfxinit, or how to hack around in their Kconfigs to disable it, because unlike everything else you need to build coreboot there seems to be no way to get an ADA toolchain from crossgcc. All the problems we have been discussing in this thread can be worked around easily (for 99% of people's host machines, at least) by putting a simple CC=util/crossgcc/xgcc/bin/x86_64-elf-gcc on the command line, but if you try to build in an environment that doesn't bring its own ADA compiler you're just SOL. So I really don't think vboot deserves the award for most cumbersome submodule right now.) _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
