Commit Graph

2327 Commits (master)

Author SHA1 Message Date
Leah Rowe d7312260e7 util/nvmutil: remove excessive comments
Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-02-25 13:29:16 +00:00
Leah Rowe e348ea0381 Bump GRUB revision to add 73 security patches
You can find information about these patches here:
https://lists.gnu.org/archive/html/grub-devel/2025-02/msg00024.html

GRUB has been on a crusade as of late, to proactively audit
and fix many security vulnerabilities. This lbmk change brings
in a comprehensive series of patches that fix bugs ranging from
possible buffer overflows, use-after frees, null derefs and so on.

These changes are critical, so a revision release *will* be issued,
for the Libreboot 20241206 release series.

This change imports the following 73 patches which
are present on the upstream GRUB repository (commit IDs
matched to upstream):

* 4dc616657 loader/i386/bsd: Use safe math to avoid underflow
* 490a6ab71 loader/i386/linux: Cast left shift to grub_uint32_t
* a8d6b0633 kern/misc: Add sanity check after grub_strtoul() call
* 8e6e87e79 kern/partition: Add sanity check after grub_strtoul() call
* 5b36a5210 normal/menu: Use safe math to avoid an integer overflow
* 9907d9c27 bus/usb/ehci: Define GRUB_EHCI_TOGGLE as grub_uint32_t
* f8795cde2 misc: Ensure consistent overflow error messages
* 66733f7c7 osdep/unix/getroot: Fix potential underflow
* d13b6e8eb script/execute: Fix potential underflow and NULL dereference
* e3c578a56 fs/sfs: Check if allocated memory is NULL
* 1c06ec900 net: Check if returned pointer for allocated memory is NULL
* dee2c14fd net: Prevent overflows when allocating memory for arrays
* 4beeff8a3 net: Use safe math macros to prevent overflows
* dd6a4c8d1 fs/zfs: Add missing NULL check after grub_strdup() call
* 13065f69d fs/zfs: Check if returned pointer for allocated memory is NULL
* 7f38e32c7 fs/zfs: Prevent overflows when allocating memory for arrays
* 88e491a0f fs/zfs: Use safe math macros to prevent overflows
* cde9f7f33 fs: Prevent overflows when assigning returned values from read_number()
* 84bc0a9a6 fs: Prevent overflows when allocating memory for arrays
* 6608163b0 fs: Use safe math macros to prevent overflows
* fbaddcca5 disk/ieee1275/ofdisk: Call grub_ieee1275_close() when grub_malloc() fails
* 33bd6b5ac disk: Check if returned pointer for allocated memory is NULL
* d8151f983 disk: Prevent overflows when allocating memory for arrays
* c407724da disk: Use safe math macros to prevent overflows
* c4bc55da2 fs: Disable many filesystems under lockdown
* 26db66050 fs/bfs: Disable under lockdown
* 5f31164ae commands/hexdump: Disable memory reading in lockdown mode
* 340e4d058 commands/memrw: Disable memory reading in lockdown mode
* 34824806a commands/minicmd: Block the dump command in lockdown mode
* c68b7d236 commands/test: Stack overflow due to unlimited recursion depth
* dad8f5029 commands/read: Fix an integer overflow when supplying more than 2^31 characters
* b970a5ed9 gettext: Integer overflow leads to heap OOB write
* 09bd6eb58 gettext: Integer overflow leads to heap OOB write or read
* 7580addfc gettext: Remove variables hooks on module unload
* 9c1619773 normal: Remove variables hooks on module unload
* 2123c5bca commands/pgp: Unregister the "check_signatures" hooks on module unload
* 0bf56bce4 commands/ls: Fix NULL dereference
* 05be856a8 commands/extcmd: Missing check for failed allocation
* 98ad84328 kern/dl: Check for the SHF_INFO_LINK flag in grub_dl_relocate_symbols()
* d72208423 kern/dl: Use correct segment in grub_dl_set_mem_attrs()
* 500e5fdd8 kern/dl: Fix for an integer overflow in grub_dl_ref()
* 2c34af908 video/readers/jpeg: Do not permit duplicate SOF0 markers in JPEG
* 0707accab net/tftp: Fix stack buffer overflow in tftp_open()
* 5eef88152 net: Fix OOB write in grub_net_search_config_file()
* aa8b4d7fa net: Remove variables hooks when interface is unregisted
* a1dd8e59d net: Unregister net_default_ip and net_default_mac variables hooks on unload
* d8a937cca script/execute: Limit the recursion depth
* 8a7103fdd kern/partition: Limit recursion in part_iterate()
* 18212f064 kern/disk: Limit recursion depth
* 67f70f70a disk/loopback: Reference tracking for the loopback
* 13febd78d disk/cryptodisk: Require authentication after TPM unlock for CLI access
* 16f196874 kern/file: Implement filesystem reference counting
* a79106872 kern/file: Ensure file->data is set
* d1d6b7ea5 fs/xfs: Ensuring failing to mount sets a grub_errno
* 6ccc77b59 fs/xfs: Fix out-of-bounds read
* 067b6d225 fs/ntfs: Implement attribute verification
* 048777bc2 fs/ntfs: Use a helper function to access attributes
* 237a71184 fs/ntfs: Track the end of the MFT attribute buffer
* aff263187 fs/ntfs: Fix out-of-bounds read
* 7e2f750f0 fs/ext2: Fix out-of-bounds read for inline extents
* edd995a26 fs/jfs: Inconsistent signed/unsigned types usage in return values
* bd999310f fs/jfs: Use full 40 bits offset and address for a data extent
* ab09fd053 fs/jfs: Fix OOB read caused by invalid dir slot index
* 66175696f fs/jfs: Fix OOB read in jfs_getent()
* 1443833a9 fs/iso9660: Fix invalid free
* 965db5970 fs/iso9660: Set a grub_errno if mount fails
* f7c070a2e fs/hfsplus: Set a grub_errno if mount fails
* 563436258 fs/f2fs: Set a grub_errno if mount fails
* 0087bc690 fs/tar: Integer overflow leads to heap OOB write
* 2c8ac08c9 fs/tar: Initialize name in grub_cpio_find_file()
* 417547c10 fs/hfs: Fix stack OOB write with grub_strcpy()
* c1a291b01 fs/ufs: Fix a heap OOB write
* ea703528a misc: Implement grub_strlcpy()

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-02-19 21:26:23 +00:00
Leah Rowe 4b228c11f9 Merge pull request 'Update pico-serprog revision' (#271) from Riku_V/lbmk:master into master
Reviewed-on: https://codeberg.org/libreboot/lbmk/pulls/271
2025-02-12 21:20:17 +00:00
Riku Viitanen a8359e30b2 Update pico-serprog revision
Most importantly this should fix issues with rp2350 boards
not synchronizing properly.

Signed-off-by: Riku Viitanen <riku.viitanen@protonmail.com>
2025-02-12 22:19:11 +02:00
Leah Rowe d2cb954933 util/nvmutil: Fix bad error messages on R/W
The messages didn't really make sense.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-02-10 20:27:48 +00:00
Leah Rowe e1e515bd22 util/nvmutil: hardened pledge on help output
Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-02-07 12:31:46 +00:00
Leah Rowe ada057a865 Merge pull request 'Simplify the README' (#269) from runxiyu/lbmk:readme-simplification into master
Reviewed-on: https://codeberg.org/libreboot/lbmk/pulls/269
2025-02-02 10:32:39 +00:00
runxiyu 9ced146b47 README.md: Use newlines instead of bulleted list for docs/support links
I think newlines look better here. The indent that bullet-pointed lists
have, does not seem natural at the start of the document.

Signed-off-by: runxiyu <me@runxiyu.org>
2025-02-02 07:56:24 +00:00
Runxi Yu 266122592c
README.md: Use the EFF's page on Right to Repair 2025-02-02 15:19:26 +08:00
Runxi Yu e36aa8c5a5 README.md: Vastly simplify it 2025-02-01 00:18:31 +08:00
Runxi Yu c17f4381ce README.md: Mention SeaBIOS and U-Boot instead of Tianocore as payloads
SeaBIOS has been supported for a long time and seems to be the
"recommended" payload nowadays (though usually with GRUB too). I haven't
seen Tianocore / EDK II been mentioned in a while. U-Boot support was
added as of Libreboot 20241206-rev8.

Signed-off-by: Runxi Yu <me@runxiyu.org>
2025-02-01 00:08:44 +08:00
Leah Rowe 47eb049cb4 Merge pull request 'deps/arch: genisoimage belongs to cdrtools' (#267) from runxiyu/lbmk:master into master
Reviewed-on: https://codeberg.org/libreboot/lbmk/pulls/267
2025-01-31 08:45:01 +00:00
Runxi Yu fa9a0df245
deps/arch: genisoimage belongs to cdrtools
genisoimage is not a an AUR package as suggested by aur_notice. It is
available in the "cdrtools" package in the repositories.

References: https://archlinux.org/packages/extra/x86_64/cdrtools/
Signed-off-by: Runxi Yu <me@runxiyu.org>
2025-01-31 16:38:20 +08:00
Leah Rowe a98490573b util/nvmutil: only set mac_updated at the end
after setting the checksum too

this is functionally no different, but setting it
at the start didn't sit right with me.

it's more logically correct to set it at the end,
in case any error did not result in an exit.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-29 04:45:57 +00:00
Leah Rowe 6b9cf09ca2 restore old x230 gbe file
i accidentally committed one where i'd changed
the mac address, on a previous revision to nvmutil

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-29 04:17:07 +00:00
Leah Rowe 8a43535513 util/nvmutil: Fix bad comparison
We're checking if errno is ENOTDIR, not setting it;
the previous code would always return true, and then
set errno 0, which in the context of this code was
actually OK, so this patch makes no functional difference
in practise.

However, I'm a stickler for technical correctness. I caught
this when trying to compile with clang, because clang is
quite pedantic about checking for exactly this type of bug.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-29 04:10:52 +00:00
Leah Rowe a65a0c2f96 util/nvmutil: allow ./nvm gbe MAC
previously, if the user ran:

./nvm GBE [MAC address]

it would error, treating the MAC as a command

now if only 3 arguments are provided, and the
3rd argument ins't a valid command, it's treated
as a MAC address and validated accordingly.

this should make nvmutil easier to use, because
I imagine a lot of users forget to use setmac

there's no reason we should be so pedantic. we
should allow it to be used flexibly like this

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-29 04:07:54 +00:00
Leah Rowe 96356ce94f util/nvmutil: move "e" to swap()
we only use it there, so we should only define it
there. it's used to detect host CPU endianness.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-29 03:47:20 +00:00
Leah Rowe b1d8975959 util/nvmutil: Only read up to 4KB on larger gbe
On the 16KB and 128KB files, we still only need to
operate on 4KB at the start of each block, where the
block size is larger than 4KB.

The reason we deal with the entire 4KB block is because
the nvm words (in the 128 byte section) can define an
extended nvm area anywhere after 128 bytes, within the
128 byte block.

We could systematically read where that is being handled,
and handle it; we could then allocate less memory, and
read/write fewer bytes, but many block devices like SSDs
and flash drives have at least a 4KB erase block anyway,
so it's kinda pointless. saving memory would be nice, but
I don't really want to bloat the code.

This is a nice easy optimisation, to avoid wasting an
additional 8KB of memory when handling 16KB files, and
additional 120KB if handling 128KB files, since nf is
what determines how much memory will be allocated.

the alternative would be to use an mmap, and then we
could reasonably handle the idea above for only writing,
surgically, what we need: nvm words and extended nvm
words.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-29 03:41:55 +00:00
Leah Rowe 6821659bcb util/nvmutil: fix minor mistake (line break)
Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 18:46:36 +00:00
Leah Rowe 3bb7520f6d util/nvmutil: do setmac if only filename given
./nvm gbe.bin

with this patch, the above example does the same as:

./nvm gbe.bin setmac

now you can simply specify the gbe file, and it will
randomise the mac address within it, and update the
nvm checksum word.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 18:40:44 +00:00
Leah Rowe d94b274fd9 vendor.sh: don't error if grep -v fails
Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 06:57:30 +00:00
Leah Rowe 6ebdd3c72b vendor.sh: Don't show gbe filename on inject
it's a temporary file, so printing it may confuse
the user. hide it from the output.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 06:49:45 +00:00
Leah Rowe a08748a9ed util/nvmutil: don't say write not needed if errno
otherwise, the output is confusing

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:54:06 +00:00
Leah Rowe 6841a351eb util/nvmutil: print dump *after* modification
this way, we still get an error exit for example
when trying to invalidate an already invalid
checksum; this error exit was disabled by the
last revisions.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:50:34 +00:00
Leah Rowe da0a6c216c util/nvmutil: verbosely print the written MAC
This is for user friendliness. Otherwise, many users
might try to dump afterward if they specified a random
MAC address.

This saves the user from having to re-run with the dump
command, thus saving time for the user.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:37:02 +00:00
Leah Rowe db5879c6b5 util/nvmutil: minor cleanup in cmd_dump
Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:29:14 +00:00
Leah Rowe bd7215d1eb util/nvmutil: show nvm words written on writeGbe
Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:26:59 +00:00
Leah Rowe c70117c79c util/nvmutil: clean up readonly check on writeGbe
Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:18:38 +00:00
Leah Rowe cf5a63e65c util/nvmutil: Remove useless gbeFileChanged var
We don't need it.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:13:20 +00:00
Leah Rowe 83601aa524 util/nvmutil: reset errno if any MAC updated
instead of setting errno in the for loop, set a variable
declaring that the mac was updated, and reset errno based
on that.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:08:17 +00:00
Leah Rowe 3e86bf5ce2 util/nvmutil: reset errno when writing a MAC
if checksum verification passed, then we should reset
in case we're operating on a given part and the last
one checked was bad.

a catch-all reset is already performed in writeGbe,
but it's good to do it here too.

in practise, if the 2nd part (part 1) is what failed,
errno still wouldn't be reset.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:05:59 +00:00
Leah Rowe bcf53cc2cc util/nvmutil: show total number of bytes read
Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 05:02:54 +00:00
Leah Rowe c91cc329cf util/nvmutil: rename tbw/bw to tnw/nw
to match nr in the readGbe function

number of bytes written, and total number
of bytes written.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 04:59:17 +00:00
Leah Rowe 9060710833 util/nvmutil: err if bytes read lower than nf
same as the last change. just covering edge cases.

we will likely never trigger this error.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 04:57:32 +00:00
Leah Rowe c72f699d36 util/nvmutil: err if fewer bytes written
it will probably never happen, and this is technically
not an error condition of pread/pwrite, but we need it
to read and write that exact number of bytes, as per nf

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 04:55:05 +00:00
Leah Rowe d666f67ebe util/nvmutil: Show bytes written in writeGbe
This will be useful for future debugging, and future
work on optimisations.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 04:47:49 +00:00
Leah Rowe b2d6393ed5 util/nvmutil swap(): ensure that no overflow occurs
it wouldn't occur, on the current logic, but i wasn't
comfortable having the starting point (on little endian)
being higher than the checked endpoint, in case of
possible integer overflow as a result of future
modifications.

this is therefore a pre-emptive bug fix, because it doesn't
yet fix a bug, but it prevents a bug from being introduced.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 04:09:48 +00:00
Leah Rowe 063fef14d3 util/nvmutil: make swap() a bit clearer
don't sizecode. show the individual steps clearly.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 04:09:09 +00:00
Leah Rowe fd1bbdc96c util/nvmutil: make 0x3f checksum position a define
for code clarity

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 04:06:12 +00:00
Leah Rowe 5ddf7f251d util/nvmutil: make 128 (nvm area) a define
for code clarity

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 04:03:34 +00:00
Leah Rowe 8850acc7da util/nvmutil swap(): Only handle the nvm area
The 128-byte nvm area is all that we need to handle,
since that is the only thing we actually work on in
nvmutil, based on checksum verification; the latter
implies that bytes must be in the correct order.

The swap() function previously worked on the entire
block, e.g. 4KB on 8KB files, 8KB on 16KB files and
64KB on 128KB files, and it did this twice, so it would
have operated on anywhere between 8KB to 128KB of data.

It now only operates on 256 bytes at a maximum, or 128
bytes if only handling one block. This is a significant
performance optimisation, on big endian host CPUs.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-27 03:56:13 +00:00
Leah Rowe 49506a8832 util/nvmutil: move write checks to writeGbe
doing it in main() is messy. better do it from the
actual function. now the logic in main is clearer.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-26 08:52:26 +00:00
Leah Rowe 948377b0e7 util/nvmutil: make cmd_swap its own function again
previous audits sizecoded nvmutil.c, reducing the sloccount,
but this resulted in unreadable code.

move the swap logic (swap parts) back to its own function,
for clarity.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-26 08:48:51 +00:00
Leah Rowe 6e134c9f4b util/nvmutil: minor cleanup
SIZE_64KB no longer needed, and the malloc error
is needlessly verbose

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-26 08:03:27 +00:00
Leah Rowe 98e105ac4f util/nvmutil: allocate less memory for setchecksum
also cmd_brick

where the checksum is being corrected or bricked, we
only need to handle the 128-byte nvm area on one of
the parts

similarly, we only need to allocate half the gbe file
size when doing a copy command.

256 bytes still allocated for setmac (see previous
commit), because we verify both checksums and set both
parts if possible.

with this, nvmutil is now much more memory-efficient.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-26 07:25:26 +00:00
Leah Rowe 52e8ea57f7 util/nvmutil: Further reduce memory usage
Allocate memory based on nf instead of partsize.

nf is the number of bytes actually read from each
part of the file.

Now if the user is running setmac for example,
256 bytes of memory will be allocated regardless
of gbe file size, whereas it would have previously
allocated 8KB, 16KB or 128KB depending on the file.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-26 07:05:06 +00:00
Leah Rowe 7a7d356824 util/nvmutil: Remove unnecessary buf16 variable
We can just point to gbe[] directly, in the word macro.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-26 06:26:53 +00:00
Leah Rowe cdf23975bc util/nvmutil: Only allocate needed memory for file
We were allocating 128KB even if we only needed 8KB, for
example. It's not a lot of memory, but the principle of
the matter is that we must respect the user by not wasting
their memory.

The design of nvmutil is that it will never overflow, because
operations are mapped in memory to the exact size of the gbe
file, which can be 8KB, 16KB or 128KB, and this is enforced.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-26 06:09:04 +00:00
Leah Rowe ed45da9cae util/nvmutil: Remove unnecessary buffer
The buf variable is only used once, and only so
that we can get a pointer. We can point to buf16
instead, for the same result.

The gbe pointer (size_t) is later converter to
a char * when writing back to the file.

Signed-off-by: Leah Rowe <leah@libreboot.org>
2025-01-25 06:23:22 +00:00