-
Notifications
You must be signed in to change notification settings - Fork 10
Increase DATA_LIMIT to 64 KiB #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
This change needs to be coordinated with a change to edk2. A fixed size shared memory buffer is allocated at startup and used for each variable service call. The existing shmem buffer is 16 pages with 14 pages for data, 1 page for the name, and 1 page for other protocol overhead. If DATA_LIMIT is increased by 2 pages, the shmem buffer will also need to be. |
|
Got it, I'll look at that. But it also means that there's no compatible way to increase the variable size, as SHMEM_PAGES is hardcoded in both varstored and edk2. I'm thinking of adding a new port handler at say 0x10c that allows for dynamic setting of SHMEM_PAGES from edk2's side, up to a reasonable limit (say 32 pages). |
|
The protocol is versioned so you shouldn't need to add a new port handler (more overhead in varstored and ports are limited in quantity). The existing protocol is: I'd suggest changing it to: In varstored in Update edk2 to use the new version and make sure that varstored is updated before (or at the same time as) edk2. (There should probably also be a check in |
|
Are the edk2 side of these changes available somewhere? |
|
I have prepared a patch for the XS RPM here, but it's fairly untested: |
Signed-off-by: Tu Dinh <[email protected]>
If we grow SHMEM_SIZE any further, we should not allocate it from the stack. Remove SHMEM_SIZE which is no longer needed. Signed-off-by: Tu Dinh <[email protected]>
The hardcoded SHMEM_PAGES in protocol v1 was insufficient to meet the
WHCP System.Fundamentals.Firmware.UEFISecureBoot point 30:
Reserved Memory for Windows Secure Boot UEFI Variables. A total of at
least 128 KB of non-volatile NVRAM storage memory must be available for
NV UEFI variables (authenticated and unauthenticated, BS and RT) used by
UEFI Secure Boot and Windows, and the maximum supported variable size
must be at least 64 KB.
Protocol v2 uses the following communication buffer format:
uint32_t version = 2;
uint32_t shmem_pages;
uint32_t cmd;
(command-specific data)
Protocol v2 caps the maximum page count to 32, allowing a theoretical
maximum DATA_LIMIT of 122880 bytes. However, limit the current v2
DATA_LIMIT to 65536, while keeping the ability to transparently increase
this limit later.
The introduction of v2 has a few implications:
* DATA_LIMIT is now broken into 2 values: a per-protocol limit and a
global limit DATA_LIMIT_MAX, imposed on all variables regardless of
protocol.
* It is now possible to use v2 to set/append to a variable in a way that
makes it unreadable from v1 (as it'd be bigger than DATA_LIMIT_V1). As
a result, get_variable can no longer assume that current variable size
will be smaller than the comm buffer size (TODO: verify safety via
test)
test_set_variable_resource_limit was adapted for v2 to only insert 1
initial variable, since 2 variables is too big for TOTAL_LIMIT.
Signed-off-by: Tu Dinh <[email protected]>
Signed-off-by: Tu Dinh <[email protected]>
This lets valgrind precisely check buffer accesses based on the protocol version being used. Signed-off-by: Tu Dinh <[email protected]>
UEFI 2.8 and newer specifies that GetVariable() returns variable attributes even if the EFI_BUFFER_TOO_SMALL error is encountered. Return this value in v2 protocol. Note that in v2, in the EFI_BUFFER_TOO_SMALL case, `attributes` comes after `data_len` to be compatible with v1. Signed-off-by: Tu Dinh <[email protected]>
Signed-off-by: Tu Dinh <[email protected]>
Previously, any data_len passed to GetVariable() would be accepted, as there exists only one single DATA_LIMIT. Now that buffer size is somewhat decoupled from variable size, check data_len as if it's untrusted. Fully decoupling buffer and variable size would require further work and potentially another version bump. Add a basic test for nr_pages in v2. Signed-off-by: Tu Dinh <[email protected]>
Signed-off-by: Tu Dinh <[email protected]>
WHCP System.Fundamentals.Firmware.UEFISecureBoot point 30 dictates:
Rebased on #27.