Page MenuHomeFreeBSD

Set geom_eli module version
AbandonedPublic

Authored by lattera-gmail.com on Mar 1 2018, 7:57 AM.

Details

Summary

The geom_eli module lacks version information. When a kernel has the geom_eli module compiled in, but bsdinstall told the loader to load the module via loader.conf (for example, full-disk encrypted setups), the kernel will reload the module later on in the boot process, causing geli to wipe the cached passphrase and prompt the user a second time for it. Setting the module version should tell the kernel to only load it once.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

allanjude requested changes to this revision.Mar 1 2018, 1:21 PM
allanjude added a subscriber: allanjude.
allanjude added inline comments.
sys/geom/eli/g_eli.c
1335

Rather than '1' as the value here, I think you want to use 'G_ELI_VERSION' from g_eli.h

When we upgrade GELI to version 8 in the future, this will make a difference.

This revision now requires changes to proceed.Mar 1 2018, 1:21 PM
sys/geom/eli/g_eli.c
1335

Good call. I'll update the revision with that change.

Use G_ELI_VERSION from g_eli.h as the module version.

lattera-gmail.com marked 2 inline comments as done.Mar 1 2018, 1:26 PM
oshogbo accepted this revision.Apr 15 2018, 8:46 PM

LGTM.

Friendly ping. :)

imp added a comment.May 6 2018, 5:57 PM

So there's already this at the end of the file:

MODULE_VERSION(geom_eli, 0);

from r332387.

sys/geom/eli/g_eli.c
1335

I don't think that this version is the same as the G_ELI_VERSION...

In D14553#322971, @imp wrote:

So there's already this at the end of the file:
MODULE_VERSION(geom_eli, 0);
from r332387.

Looks like that commit was made after this patch was posted for review. Given that it now includes MODULE_VERSION (but without using G_ELI_VERSION), we can probably just keep with what was committed. Should I close this review?

imp added a comment.May 7 2018, 10:59 PM

I think you can close this.

lattera-gmail.com abandoned this revision.May 7 2018, 11:02 PM

Closing this review since the issue has been addressed with a different commit.