ii used to rely on errno for exit status, but this was flawed.
when removing it, i neglected to adjust the actual error exits,
setting errno accordingly. this patch should fix it. this is
important for scripts that use nvmutil, which may rely on its
error status upon exit.
Signed-off-by: Leah Rowe <leah@libreboot.org>
This reverts commit fedeb6ecd8.
this, along with several other updates, have been reverted.
dell 3050 micro had boot issues, on the update. i therefore
revert the recent revision update, pending further investigation.
thanks to sirlami on irc for pointing this out.
due to lbmk blob policy, we must not distribute
fsp except as the full, concatenated binary, thus
complying with intel's license. it is removed from
builds before release, re-inserted via vendor
inject scripts in the usual way.
a while ago, coreboot updated fsp but i had to keep
it on the earlier version for this board, lest old
releases no longer match vendor insertion; the new
fsp version also wasnet well tested, and didn't seem
to contain any changes reported that pertained to
our use of it.
the fsp headers do not match the fsp binary in use,
causing boot issues for users on topton x2e n150.
this patch should fix the issue, as reported by
sirlami who is the one who found and first tested
this fix; i'm simply changing the configuration,
as per sirlami's guidance.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this version is more tested. i'll merge _decode back
once it's better tested. it contains a lot of invasive
changes, whereas recv is much closer to the original GNU
code that i inherited, that i also know works quite well.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i should probably test musl as well, on linux
libreboot-utils is stable on the glibc systems i tested
with linux. it is quite buggy on bsd systems.
it's irresponsible to let users compile this until i've
properly tested the code. putting this error in for now.
i made lbmk use the old nvmutil version for now, and retro
fitted several improvements to i/o there from lbutils,
changes that i know are stable on bsd.
Signed-off-by: Leah Rowe <leah@libreboot.org>
adapted from the changes made in lbutils
i'm just patching this crappy code. lbutils doesn't
work properly on openbsd yet, and i just want nvmutil
to work properly there, so i'm using the old code for
now, on openbsd.
Signed-off-by: Leah Rowe <leah@libreboot.org>
don't fix it for now. this version was buggy.
i'm only using nvmutil for now, until i properly
fix all the memory issues in lbutils on openbsd.
Signed-off-by: Leah Rowe <leah@libreboot.org>
same fix i did for lbutils
the nvmutil version is the same one used before
lbutils was introduced. just before.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i'm trying to make nvmutil work on openbsd. the new code
in lbutils is a bit buggy, likely somewhere in mkhtemp.
i'm still debugging it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
check if above or equal to zero, except where
counterindicated. this is the usual way on unix,
where a command returns -1 on error, or above/equal
to zero on success.
Signed-off-by: Leah Rowe <leah@libreboot.org>
in the linux fast path, we are universally overriding
errno with EEXIST, which pollutes errno in case of
debugging under fault condition. this is inconsistent
with posix and also leah.
Signed-off-by: Leah Rowe <leah@libreboot.org>
See the commit message in the patch:
Before this patch, hotplugging only worked to replace drives (if you
tried to plug a drive into a SATA port that no drive was plugged in to
at boot, it wouldn't be detected) and you'd have to manually rescan
the bus (echo "- - -" > /sys/class/scsi_host/host*/scan) to make
plugs/unplugs get detected by the operating system.
Now, hotplugging works for all ports (tested and working on Supermicro
X11SSH-LN4F) and there's no need to manually rescan (it sometimes
takes a few seconds for unplugs to be detected, but plugs are detected
instantly).
Also submitted upstream as https://review.coreboot.org/c/coreboot/+/91824
not indicated. the way we use it is basically like
stat, to check that a file exists / is a file.
just err the fuck out
nuance: SETLK is non-blocking (no wait).
we should loop on SETLKW, but we don't use that.
in this codebase, we use SETLK for locking a
tmpfile, but because of race conditions and
wanting to make another file quickly, we just
try again with a newly generated name, with a
certain number of retries, so we justt use SETLK
Signed-off-by: Leah Rowe <leah@libreboot.org>
call it rw_exact, so that it's closer to
the name rw. it matches naming more closely;
the alternative was to call rw rw_file
but read/write can handle more than just files!
Signed-off-by: Leah Rowe <leah@libreboot.org>
even with a timer, it's possible that on a buggy system,
we may keep writing even though the outcome is zero. if
a system comes back with zero bytes written, that is a
fatal bug and we should stop.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the actual warn flags are still there.
leaving Werror in production is ill advised.
i can (and will) still fix build errors as
i see them.
as a result of this, i now also see more info
when i type:
make strict
(this uses clang with -Weverything)
Signed-off-by: Leah Rowe <leah@libreboot.org>
rw is enough. i unified everything there.
next commit will remove rw_type and instead
run positional i/o depending on whether the
offset is zero. i'm simplifying the API a lot.
Signed-off-by: Leah Rowe <leah@libreboot.org>
add options for building with urandom+openat and
arc4+openat. useful for emulating a bsd / old linux
environment in modern linux distros, for portability
testing.
these options are not recommended for everyday use.
just use make without any special options, and the
code has build-time OS detection for features like
randomisation/openat2.
Signed-off-by: Leah Rowe <leah@libreboot.org>
these can be set explicitly in the compiler flags,
e.g.
make CC="cc -DUSE_OPENAT=1 -DUSE_URANDOM=1"
these options, if set to 1, will cause you to use
the code as if it were running on non-linux systems
such as openbsd. of course, some differences will
still exist, but this is useful for portability
testing when compiling on linux.
Signed-off-by: Leah Rowe <leah@libreboot.org>
and remove manual prototypes; fchmod, realpath
and so on rely on the _XOPEN_SOURCE macro.
the POSIX macro wasn't needed: _XOPEN_SOURCE
is sufficient.
Signed-off-by: Leah Rowe <leah@libreboot.org>
otherwise, i will end up with a mess like the
one i recently fixed.
we always want to use correct C. the current
spec is set to c99, with -pedantic turned on.
flags now:
-Os -Wall -Wextra -std=c99 -pedantic -Werror
if you do: make hell, you get (uses clang):
-Os -Wall -Wextra -std=c99 -pedantic -Werror -Weverything
i initially loosened up the Makefile rules, so
that the code would be more "portable", but
every compiler worth caring about has these
flags, and turning them on is advisable,
especially pedantic and -std, because you want
to have some guarantee that the compiler is
generating correct code; if the standard is
left ambiguous, you could be introducing subtle
bugs when people compile it, because who knows
what spec the compiler is using?
Signed-off-by: Leah Rowe <leah@libreboot.org>
i wasn't using strict mode enough in make:
make strict
now it compiles cleanly. mostly removing
unused variables, fixing implicit conversions,
etc.
Signed-off-by: Leah Rowe <leah@libreboot.org>
In file included from /home/user/lbmk/src/coreboot/fam15h/util/cbfstool/partitioned_file.h:19,
from /home/user/lbmk/src/coreboot/fam15h/util/cbfstool/partitioned_file.c:16:
/home/user/lbmk/src/coreboot/fam15h/util/cbfstool/common.h:34:16: error: expected ‘)’ before ‘__attribute__’
34 | #define unused __attribute__((unused))
| ^~~~~~~~~~~~~
In file included from /home/user/lbmk/src/coreboot/fam15h/util/cbfstool/common.h:25:
/home/user/lbmk/src/coreboot/fam15h/src/commonlib/include/commonlib/helpers.h:137:40: error: expected identifier or ‘(’ before ‘)’ token
137 | #define __unused __attribute__((unused))
^ this removes that error
Signed-off-by: Leah Rowe <leah@libreboot.org>
but i can't write a generic function for this,
because fcntl is a variadic function, so wrapping
cannot be done cleanly.
Signed-off-by: Leah Rowe <leah@libreboot.org>
otherwise, we invoke the state machine in weird
conditions, where some pointers may not be initialised.
we could handle this properly, but why?
therefore, the errhook is called after the argc
check.
this patch fixes a Speicherzugriffsfehler that
i got while running nvmutil with below 3 arguments
Signed-off-by: Leah Rowe <leah@libreboot.org>
where possible, try not to clobber sys errno. override
it only when relatively safe.
also: when a syscall succeeds, it may set errno. this
is rare, but permitted (nothing specified against it
in specs, and the specs say that errno is undefined
on success).
i'm not libc, but i'm wrapping around it, so i need
to be careful in how i handle the errno value.
also:
i removed the requirement for directories to be
executable, in mkhtemp.c, because this isn't required
and will only break certain setups.
in world_writeable and sticky, i made the checks stricter:
the faccessat check was being skipped on some paths, so
i've closed that loophole now.
i also generally cleaned up some code, as part of the errno
handling refactoring, where it made sense to do so, plus a
few other bits of code cleanup.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the portable version was written for fun, but
it's bloat, and makes the code hard to read.
every unix since about 2005 has these functions.
Signed-off-by: Leah Rowe <leah@libreboot.org>
a non-blocking file descriptor could be used while
errno is set to these. this would create an infinite
loop. it's better that we only allow EINTR.
Signed-off-by: Leah Rowe <leah@libreboot.org>
some io syscalls may set errno on success. this patch
honours that. we try to preserve caller errno, but it
is important for debugging not to clobber it.
if fs_err_retry errs, then we don't reset errno.
if fs_err is successful but errno wasn't set, we
restore caller errno. this is done by setting errno
to zero in callers, which also restore caller errno.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i previously used error status and set return values
indirectly. i still do that, but where possible, i
also now return the real value.
this is because these string functions can no longer
return with error status; on error, they all abort.
this forces the program maintainer to keep their code
reliable, and removes the need to check the error status
after using syscalls, because these libc wrappers mitigate
that and make use of libc for you, including errors.
this is part of a general effort to promote safe use
of the C programming language, especially in libreboot!
Signed-off-by: Leah Rowe <leah@libreboot.org>
on the conditions where these functions encounter
an unexpected error, we currently return -1
this means that the caller must check. which means
the caller won't check. nobody does. i often forget.
force the caller (me) to be correct, instead.
the current calling convention is that the real
return value is stored in a pointer, provided inside
the function signature, on a given string function,
and the function's return value is merely an indicator.
this calling convention is retained for now; the next
patch will change it, such that the real value is also
the function's return value. this is more flexible.
Signed-off-by: Leah Rowe <leah@libreboot.org>
operate per word, not per byte
this is also done on sdup, which uses a slightly
inefficient method: the new string allocation is
that of the maximum size, rather than what we
need. for example, if you wanted a 20 character
string (21 including null), you would still allocate
4096 bytes if that was the maximum length.
it's a bit naughty, and i have half a mind to
keep sdup on the old implementation, but i'll leave
it be for now.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i added a fake -t option, which doesn't actually
read optarg, so that -t usage can just override
the normal template. mkhtemp isn't ready for
distros yet, but it's ready for lbmk.
i hacked the makefile to also copy the binary to
mktemp, and i set PATH in lbmk so that this binary
is used insttead of the one on your system.
that way, upstream projects use it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
delete tmpfiles after operation. fixes a bug where
tmpfiles are left behind after running the dump
command.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i forgot to do this!
with this, I/O should be bullet proof now.
i already loop this on other I/O commands.
Signed-off-by: Leah Rowe <leah@libreboot.org>
spoiler alert: it's slow as molasses
part 2 will be presented at a later date
(yes, please don't fill 8GB of memory with
random data and hexdump it)
Signed-off-by: Leah Rowe <leah@libreboot.org>
the functions no longer return errors, so i don't
need to handle them.
furthermore, the handling in state.c is redundant,
so i've removed that too.
Signed-off-by: Leah Rowe <leah@libreboot.org>
make it more reliable; it can't segfault
now, under any circumstance. not even
once.
the problem arised when lbsetname was not
called in a program, before calling the
function: lbgetprogname. a segfault would
occur, due to it being NULL.
not every os/libc has getprogname, so i have
my own implementation.
Signed-off-by: Leah Rowe <leah@libreboot.org>
not c90
i use stdint now on a few files. i had this
idea in my head to use C89 for some reason,
but this is pointless.
c99 however is worthy as a minimum, because
for example, compilers like tcc will adhere
to its spec (for the most part), so this is
the minimum reasonable requirement on modern
unix systems.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this is a more generic one that i implemented
for "lottery.c" (which is really just a tester
of my rset function in lib/rand.c)
i could probably actually write a full hexdump
program in libreboot-utils to be honest.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i currently return pointers to these, without copying.
they can fade because of this. make them static, since that
is what they should be anyway.
Signed-off-by: Leah Rowe <leah@libreboot.org>
free can take a null, that's fine, but my pointer
to the pointer being freed should not be null. that
is a bug.
Signed-off-by: Leah Rowe <leah@libreboot.org>
remove close_warn and close_no_err
make close_on_eintr a void, and abort
on error instead of returning -1.
a failed file closure is a world-ending
event. burn accordingly.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i now use a singleton hook function per program:
nvmutil, mkhtemp and lottery
call this at the startup of your program:
(void) errhook(exit_cleanup);
then provide that function. make it static,
so that each program has its own version.
if you're writing a program that handles lots
of files for example, and you want to do certain
cleanup on exit (including error exit), this can
be quite useful.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i no longer care about openbsd 5.9. we assume unveil
is available, as has been the case for the past 12
years.
i use wrappers for unveil and pledge, which means that
i call them on every os. on OSes that don't have these,
i just return. it's somewhat inelegant, but also means
that i see errors more easily, e.g. misnamed variables
inside previous ifdef OpenBSD blocks.
Signed-off-by: Leah Rowe <leah@libreboot.org>
yes, a common thing in C programs is one or all
of the following:
* use after frees
* double free (on non-NULL pointer)
* over-writing currently used pointer (mem leak)
i try to reduce the chance of this in my software,
by running free() through a filter function,
free_if_not_null, that returns if a function
is being freed twice - because it sets NULL
after freeing, but will only free if it's not
null already.
this patch adds two functions: smalloc and vmalloc,
for strings and voids. using these makes the program
abort if:
* non-null pointer given for initialisation
* pointer to pointer is null (of course)
* size of zero given, for malloc (zero bytes)
i myself was caught out by this change, prompting
me to make the following fix in fs_dirname_basename()
inside lib/file.c:
- char *buf;
+ char *buf = NULL;
Yes.
Signed-off-by: Leah Rowe <leah@libreboot.org>
clamp rand to eliminate modulo sampling; high
values on the randomisation will bias the result.
not really critical for mac addresses, but there's
no reason not to have this. this patches reduces
the chance that two libreboot users will generate
the same mac addresses!
Signed-off-by: Leah Rowe <leah@libreboot.org>
(for real pwrite/pread. don't use the compatibility
one - it works perfectly, but using it is pointless
and may have unknown bugs, even though i know it's
probably perfect)
Signed-off-by: Leah Rowe <leah@libreboot.org>
concatenate an arbitrary number of strings,
pointed to by char **
i'll use this and the next function, dcatn,
in an upcoming feature planned for mkhtemp.
Signed-off-by: Leah Rowe <leah@libreboot.org>
make it more efficient. much lower rejection
rate now, about 2-5%. deal with bias, but also
get numbers in bulk. not too many.
i'd say this is about right in terms of performance
balance. 64 bytes == 8 large integers.
Signed-off-by: Leah Rowe <leah@libreboot.org>
build-time option. do not allow fallback; on
a system where getrandom is used, it should
be used exclusively.
on some systems, getrandom may not be available,
even if they have a newer kernel.
Signed-off-by: Leah Rowe <leah@libreboot.org>
This will also be used in lbmk itself at some point,
which currently just uses regular mktemp, for tmpdir
handling during the build process.
Renamed util/nvmutil to util/libreboot-utils, which
now contains two tools. The new tool, mkhtemp, is a
hardened implementation of mktemp, which nvmutil
also uses now. Still experimental, but good enough
for nvmutil.
Mkhtemp attempts to provide TOCTOU resistance on
Linux, by using modern features in Linux such as
Openat2 (syscall) with O_EXCL and O_TMPFILE,
and many various security checks e.g.
inode/dev during creation. Checks are done constantly,
to try to detect race conditions. The code is very
strict about things like sticky bits in world writeable
directories, also ownership (it can be made to bar even
root access on files and directories it doesn't own).
It's a security-first implementation of mktemp, likely
even more secure than the OpenBSD mkstemp, but more
auditing and testing is needed - more features are
also planned, including a compatibility mode to make
it also work like traditional mktemp/mkstemp. The
intention, once this becomes stable, is that it will
become a modern drop-in replacement for mkstemp on
Linux and BSD systems.
Some legacy code has been removed, and in general
cleaned up. I wrote mkhtemp for nvmutil, as part of
its atomic write behaviour, but mktemp was the last
remaining liability, so I rewrote that too!
Docs/manpage/website will be made for mkhtemp once
the code is mature.
Other changes have also been made. This is from another
experimental branch of Libreboot, that I'm pushing
early. For example, nvmutil's state machine has been
tidied up, moving more logic back into main.
Mktemp is historically prone to race conditions,
e.g. symlink attacks, directory replacement, remounting
during operation, all sorts of things. Mkhtemp has
been written to solve, or otherwise mitigate, that
problem. Mkhtemp is currently experimental and will
require a major cleanup at some point, but it
already works well enough, and you can in fact use
it; at this time, the -d, -p and -q flags are
supported, and you can add a custom template at
the end, e.g.
mkhtemp -p test -d
Eventually, I will make this have complete parity
with the GNU and BSD implementations, so that it is
fully useable on existing setups, while optionally
providing the hardening as well.
A lot of code has also been tidied up. I didn't
track the changes I made with this one, because
it was a major re-write of nvmutil; it is now
libreboot-utils, and I will continue to write
more programs in here over time. It's basically
now a bunch of hardened wrappers around various
libc functions, e.g. there is also a secure I/O
wrapper for read/write.
There is a custom randomisation function, rlong,
which simply uses arc4random or getrandom, on
BSD and Linux respectively. Efforts are made to
make it as reliable as possible, to the extent
that it never returns with failure; in the unlikely
event that it fails, it aborts. It also sleeps
between failure, to mitigate certain DoS attacks.
You can just go in util/libreboot-utils and
type make, then you will have the nvmutil and
mkhtemp binaries, which you can just use. It
all works. Everything was massively rewritten.
Signed-off-by: Leah Rowe <leah@libreboot.org>
Adds libreboot support for the Dell OptiPlex 3040 Micro based on the
OptiPlex 3050 Micro (same Skylake H110 PCH-H platform). Key differences:
DDR3L SODIMMs, Pentium G4400T-class CPUs (Skylake only), Realtek ALC3234
HDA, and Boot Guard neutralization via deguard.
Tested and booted on hardware.
Signed-off-by: Todd Baker <todd_baker@student.uml.edu>
err if buf NULL, len -1
also getrandom may return fewer bytes, so
loop that too.
why can't linux be like bsd? bsd is:
arc4random_buf(buf, len);
no checks needed. it never errs.
Signed-off-by: Leah Rowe <leah@libreboot.org>
openbsd 2.1 has arc4random, which we detect here.
arandom was apparently added much later, so this
is dead code. remove it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we still fall back to the old /dev/urandom read
on older linux, via runtime detection (ENOSYS).
getrandom is better, because it guarantees entropy
via blocking, and works even when /dev/urandom
is unavailable.
it has the same practical benefit as arc4random,
which i use on bsd. linux can have arc4random,
but not every linux libc has it, so it's better
to use getrandom on linux.
older linux will fall back to /dev/urandom
Signed-off-by: Leah Rowe <leah@libreboot.org>
more random characters
i added support for higher than the standard 6
characters so i can go nuts
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
Signed-off-by: Leah Rowe <leah@libreboot.org>
200 retries, not 100.
and open with O_NOFOLLOW and O_CLOEXEC
check X on mkstemp
support more than 6 X in mkstemp
make PATH_LEN 4096
1024 is a bit low
make default mkstemp length 4096
Signed-off-by: Leah Rowe <leah@libreboot.org>
i had this idea in my head of later porting this
to k&r c for fun. but screw it.
compiling on everything since 1989 is enough
Signed-off-by: Leah Rowe <leah@libreboot.org>
handle init in xstatus()
it's now a singleton design
also tidied up some other code
also removed todo.c. bloat.
will do all those anyway.
too much change. i just kept
touching the code until it
looked good
Signed-off-by: Leah Rowe <leah@libreboot.org>
question mark respects environmental variables
but isn't portable
you can just pass as argument on the command line
question mark is more useful for build systems,
but i'm not really bothered. the old way works.
Signed-off-by: Leah Rowe <leah@libreboot.org>
make a singleton function instead
now there are technically no global variables,
so i can more easily start splitting this up
into multiple linked programs
Signed-off-by: Leah Rowe <leah@libreboot.org>
must be world writeable and not have sticky bits
a bit theoretical, but we're also reading TMPDIR,
which could be anything
due to how this is called, it defaults back to /tmp
if null is returned, so itt's safe
Signed-off-by: Leah Rowe <leah@libreboot.org>
set it really high though, so it's still
basically reliably
an EINTR/EAGAIN storm could cause problems
in prw()
Signed-off-by: Leah Rowe <leah@libreboot.org>
otherwise, a stale descriptor could be manipulated
easily by an attacker over time
very theoretical to be honest
Signed-off-by: Leah Rowe <leah@libreboot.org>
the actual cat function just writes to stdout
we need only check that the input is null, which
i've now done.
Signed-off-by: Leah Rowe <leah@libreboot.org>
where feasible, don't assign them at declaration
this is especially important for the next change
i'm working on
Signed-off-by: Leah Rowe <leah@libreboot.org>
call it sooner. set new_state afterward.
i had to uncouple nv from some functions
for this, and i also added some extra
checks especially at exit, about whether
to touch nv (whether it is initialised)
Signed-off-by: Leah Rowe <leah@libreboot.org>
yes, this begins the next phase of nvmutil:
remove global status in functions that should be
generic, and make functions that are not generic,
generic. make everything as re-useable in a library
as possible.
most of the program is error control, as it should
be, but much of it is mixed in with functions
that really should just be split up for libraries.
so that is what i'm now beginning.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i still use a global variable, but now only
one, which is a structure containing the
state of the entire program
now i can easily start modifying it to make
functions generic, and then i can start
making parts of it into easy libraries
Signed-off-by: Leah Rowe <leah@libreboot.org>
for now still actually global, but i'm gradually
putting variables into a single global stucture
which will then allow me to make everything
local, which would then allow me to start
splitting up the program and modularising it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we assume the fallback will be rare, so now we
make the mix static and keep xoring it, on the
theory that the number of failures on urandom
will be random, and tthat the fallback may only
apply once or twice in thousands of calls.
the time jitter is adjusted; rather than judge
the difference between two points close to each
other in time, we judge tthe randomness in
difference of time elapsed. this mitigates fast
CPUs being very fast and introducing rounding
errors, and also improves performonce on much
slower CPUs
Signed-off-by: Leah Rowe <leah@libreboot.org>
i was being cute earlier, but the rewrite
defeats the purpose of atomic file handling
in nvmutil, by not actually renaming! it was
more like, doing an actual copy, which meant
that corruption is likely during power loss
i've commented the code because i may
use it in a library in the future.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this improves reliability, making it more
likely that data actually gets synced,
since fsync can return -1 with EINTR,
indicating that a re-try should be
attempted.
Signed-off-by: Leah Rowe <leah@libreboot.org>
arandom probably isn't available on super old obsd right??????
rather, unveil isn't. on systems that have arandom
yet we should not unveil something that may not
exist on modern systems
just don't unveil arandom, and don't check arandom
if unveil is enabled
Signed-off-by: Leah Rowe <leah@libreboot.org>
some systems may not even have it
works with /dev/fd (bsd/mac etc)
works with linux (/proc/self/fd)
and falls back on super old systems
that have neither
Signed-off-by: Leah Rowe <leah@libreboot.org>
settting it to -Werror is wrong, should set
it not -Werror.
however, put the WERROR variable in the make
command. that way, i could test with
make WERROR=-Werror
Signed-off-by: Leah Rowe <leah@libreboot.org>
try a few more times until success
explicitly return EEXIST when needed
we try multiple times and check more
thoroughly if a file exists, thus
reducing the risk of race conditions
Signed-off-by: Leah Rowe <leah@libreboot.org>
not portable. some old systems don't have it,
or handle it very poorly
unsigned long is a reasonable way to refer
to indexes inside pointters
Signed-off-by: Leah Rowe <leah@libreboot.org>
if the global file is created on a different file
system than the gbe file, unveil would trigger an
abort trap, since we rely on created a second
temporary file, whose path we can't know ahead
of time.
i could get rid of unveil, or unveil a directory,
but neither is acceptable. just use localtmp on
openbsd. a temporary file is created next to
the gbe file, in the same directory.
Signed-off-by: Leah Rowe <leah@libreboot.org>
now the custom fallback code is very unlikely
to ever actually be used, on any system,
except really old systems.
Signed-off-by: Leah Rowe <leah@libreboot.org>
-Werror removed, for older compilers
actual warnings still there
-std is configurable now
e.g.
make CSTD=-c90
make CSTD=-c99
Signed-off-by: Leah Rowe <leah@libreboot.org>
i have urandom again. it's enough
the fallback rand implementation
is used if needed
now i don't have to worry about any
weird version of unix from 1992 and
deal with weird hacks. in fact, with
this change, my code will probably
compile on irix now
Signed-off-by: Leah Rowe <leah@libreboot.org>
i had to loosen the pledges for the new i/o
framework, which needs more permissions
as a result, i can now open urandom in
this function statically, rather than
in nvmutil's control logic
and because of that, it's less buggy now
arc4random is disabled on linux by default,
because it's not universally available
on all libc, and only since about 2022
in some glibc versions
better for portability to let linux users
justt use urandom
the new logic is different. now it falls
back to rand per-byte, but in practise
it almost never will.
Signed-off-by: Leah Rowe <leah@libreboot.org>
linux only had it since 2022.
lots of people will complain if i leave this enabled.
not all libc have it either
Signed-off-by: Leah Rowe <leah@libreboot.org>
i simplified it in the last commits, but i sttill
need this loop to properly handle parts
otherwise yeah, all it's doing is copying a file
verbatim. duh.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we now read twice, verify the two, to make sure
one read isn't faulty
we operate on a tmp file, then rename back. this
reduces the risk of power cuts corrupting data
we properly verify the contents that we wrote
back
inspired largely by flashprog. i wanted to have
an insanely over-engineered and extremely safe
tool that edits intel gbe nvm files
and now i have one. the only one in existence.
i'm basically writing my own libc code at this
point, to be honest. i'll probably start puttting
these functions in libraries
e.g. that tmpfile generator
Signed-off-by: Leah Rowe <leah@libreboot.org>
we were returning if verified is not off, but we
were not doing the check soon enough.
now it's clearer: just after either a reset,
or we found out offset doesn't match, we
return sooner.
otherwise, we read, and we verify again right
after. in the old code, we verified twice in
a row.
this is just more optimal, for error handling.
Signed-off-by: Leah Rowe <leah@libreboot.org>
in the last patch, i return, which then avoids
resetting the offset.
prw is very careful not to return early.
Signed-off-by: Leah Rowe <leah@libreboot.org>
otherwise, if it's -1 and errno happens to be
EINTR or EAGAIN, we might loop on what is a
real error. this bug fixes that edge case.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we already check before that rv is not negative,
and it starts at zero, but it's good to guard
it here just in case (for future re-factoring).
if rv is negative, it could convert (casted to
size_t) to a huge number (we don't want that).
Signed-off-by: Leah Rowe <leah@libreboot.org>
even with OFF_RESET, we still want some error checking.
if the check fails again immediately after, then it
suggests that another program really is modifying the
file, so we should stop.
the first check is done on the theory that another
program *was* working on it, but now isn't.
once again, this isn't perfect. use read pread/pwrite
if you need thread safety (and even then, you still
need to actually check your code)
Signed-off-by: Leah Rowe <leah@libreboot.org>
if a points to a buffer shorter than maxlen,
and the string is not null-terminated early,
the loop may read may overflow
e.g.
char buf[3] = {'a', 'b', 'c'};
xstrxcmp(buf, "abc", 50);
this is undefined behaviour, and a bug. C allows
reading past arrays only if the memory exists,
but we can't guarantee that
to fix it, we check the condition for return,
namely NULL character, before using the character
again. This avoids reading further from a multiple
times so we exit as soon as we encounter NULL
this also avoids multiple reads from memory, though
a compiler would optimise that anyway
Signed-off-by: Leah Rowe <leah@libreboot.org>
we currently reset just fine, but a partial success
where the previous offset is not the same as the
original should also be considered failure.
this patch therefore makes the return much stricter,
making the code return an error if this occurs,
which in nvmutil would then cause a program exit.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we check them in prw, but we used to rely
on prw because we called that first. no more.
it's correct to also check them here anyway,
in case i ever call another function here.
Signed-off-by: Leah Rowe <leah@libreboot.org>
avoid pointer-range overflow arithmetic. this
patch doesn't change behaviour, but makes an
overflow impossible.
Signed-off-by: Leah Rowe <leah@libreboot.org>
off is signed, so converting that to unsigned
is better than converting rc (unsigned)
to signed. i had the right idea, but got
it wrong in the earlier version. this
should fix potential overflow issues.
Signed-off-by: Leah Rowe <leah@libreboot.org>
our fallback pwrite/pread behaviour still does not
properly replicate the safety of real pwrite/pread
i intend to put this i/o code into a library for use
in other programs; nvmutil is single-threaded so
this change is largely redundant (but can't hurt)
Signed-off-by: Leah Rowe <leah@libreboot.org>
rw_file_once was doing what rw_file_exact should be
doing
_once does what it says: once
we were passing an offset (rc) to it that it was not
meaningfully using.
this makes the code now more robust, especially if
we later swap out or break _once - then we don't
get weird behaviour (if there is a regression).
Signed-off-by: Leah Rowe <leah@libreboot.org>
we assert now that ulong is the size of a pointer,
therefore we know that it can fit a pointer reliably.
this code is written for c90 spec so lacks uintptr
Signed-off-by: Leah Rowe <leah@libreboot.org>
not available on older systems. can just pass
the relevant flag in the compiler:
HAVE_ARC4RANDOM_BUF=0 at build time if you need
the fallback.
Signed-off-by: Leah Rowe <leah@libreboot.org>
too over engineered and cumbersome.
the new security in prw() makes it brittle,
and i'd rather not move checks outside of it.
the fallback rand is random enough.
Signed-off-by: Leah Rowe <leah@libreboot.org>
write all at once, then sync all at once,
then verify all at once.
this increases the chancce that all data
gets written first, in the case of power
less, because fsync may take a while on
some systems.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we already covered this in prw() which is
what ultimately gets called, but still.
it's logically correct not to check it here.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the cat function can be greatly simplified
handle it conditionally, because not all
functions should use it
Signed-off-by: Leah Rowe <leah@libreboot.org>
move the gbe-specific parts out of it
what remains is a relatively generic
function; a very conservative implementation,
wrapping around libc functions but with
a few additional safety checks.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this still gets done from rw_once, but
it's generic enough that we want it in
our prw() wrapper function.
Signed-off-by: Leah Rowe <leah@libreboot.org>
most of it can be done in rw_file_once
truly general checks have been moved to prw(),
so that the function is more general purpose.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this way, we now have a universal function
that is reusable elsewhere, with the same
redundancy. the rw_once and rw_exact functions
still get this redundancy, through prw
Signed-off-by: Leah Rowe <leah@libreboot.org>
we can just fall through to nrw and decide
what function ta call there - either read/write
immediately and return, or fall back to the
portable positional implementation.
this also means we don't have to call io_args
in every function, since everything now runs
through prw()
Signed-off-by: Leah Rowe <leah@libreboot.org>
it can be higher than 32-bit, it's fine
the current check breaks some newer systems
accordingly, u32 becomes ux, x meaning x bits
Signed-off-by: Leah Rowe <leah@libreboot.org>
i didn't take into account partial writes, in io_args
this fixes it
unfortunately, this means i have to loosen the offset
check a bit, but it's fine
Signed-off-by: Leah Rowe <leah@libreboot.org>
re-check. very unlikely since the program doesn't run
for very long, but we have to check if the file has
changed. this is a basic check of file size.
we could probably check the contents too, but that
would be overkill.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the time difference used here could go negative, which
would overflow in the xor op on mix, leading to a biased
entropy pool. we want to ensure that they numbers do
not overflow, because here they are cast to unsigned
which would then produce very large numbers.
Signed-off-by: Leah Rowe <leah@libreboot.org>
zero never occurs, because rw_file_once never returns zero,
but only rw_file_once determines that. rw_file_exact must
handle every possible error.
right now, if that call returns zero, rw_file_exact would
have an infinite loop.
this doesn't actually happen at the moment, so this is a
preventative bug fix.
Signed-off-by: Leah Rowe <leah@libreboot.org>
properly verify the value of the arguments, with
asserts.
add simpler runtime checks in-function, on prw,
rw_file_once and rw_file_exact.
variable names in english now, and the code is
cleaner, while being functionally equivalent.
Signed-off-by: Leah Rowe <leah@libreboot.org>
for pulses, we currently use amplitude detection.
edge detection is better, because weak / low gain
signals will be more reliable. if audio is coming
in on/from a system that does automatic gain
adjustment, this once again is more robust too.
microphones and speakers (which people often use
with spkmodem if nothing else available) often
clamp amplitude, to an extent that this software
may not detect those pulses reliably that way.
so we detect slope edges instead. this causes
very little performance penalty (use of abs(),
that's about it)
however, edge detection is inherently vulnerable
to noise, so we will also detect amplitude. this
acts as an effective noise filter, while still
improving pulse detection.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this check no longer applies (never triggers)
is_signal_valid already guarantees that the separator
tone is valid.
Signed-off-by: Leah Rowe <leah@libreboot.org>
bits are currently assembled even on invalid frames. this
patch fixes that - the bug is also in the GNU version.
this reduces the chance of noise/calibration from creating
corrupt character output during operation.
Signed-off-by: Leah Rowe <leah@libreboot.org>
enforce at least two tones. this mitigates the
chance of random noise being treated as a real
tone, and reduces the chance of broken
thresholds versus freq min/max e.g.
freq min 31, max 32 and threshold 31
Signed-off-by: Leah Rowe <leah@libreboot.org>
we weren't actually using what we calculated. this patch
fixes that, thus preventing random noise / microphone
clicks, random artifacts and such from being treated
as real frames (the purpose of is_valid_signal is
partly noise suppression).
Signed-off-by: Leah Rowe <leah@libreboot.org>
in handle audio, i do the number of samples
per frame, and one more. e.g. 241 instead of
240. this bug is in the original GNU version
too. this patch fixes it.
this means that the output could slowly go
out of sync with calculated timings. the
patch fixes that. in practise, the decoder
is not that sensitive, and the code would
adjust anyway (automatic timing adjustment),
but ideally we want to not *cause* such
issues even if we mitigate them.
Signed-off-by: Leah Rowe <leah@libreboot.org>
assert integer sizes, important in this program because
we make several implicit assumptions about word sizes,
and integers need to be of a certain size.
Signed-off-by: Leah Rowe <leah@libreboot.org>
collect_separator should be select_separator, to bring
it in line with select_low_tone
this just makes the code a bit easier to read
Signed-off-by: Leah Rowe <leah@libreboot.org>
the fir filter produces stable frequencies per frame,
but learning per sample (within a frame) means we
record the same value roughly 240 times.
here, we are syncing up at just the right moment
instead, and only at that moment, this increasing
both performance and reliability.
Signed-off-by: Leah Rowe <leah@libreboot.org>
oops!!!
another mistake during refactoring. right now it
doesn't increment before being checked, so learning
can go forever in an infinite loop
Signed-off-by: Leah Rowe <leah@libreboot.org>
i conflated two separate tests in a previous change.
the silence check was defeated by still checking f
alongside it, which would be set, thus satisfying
the condition, and proceeding, which defeats the
purpose of the silence check (ignore false signal
that is actually noise) - so in the original patch
that i wrote, the extra checks actually do nothing.
this patch fixes that, and makes the logic a bit
clearer.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the tone detection currently only tracks data, not
the separator. track both instead, for improved
detection reliability.
e.g.
separator tone e.g: 9
data low tone e.g: 18
data high tone e.g: 24
two fir windows produce e.g.
freq data 9 sep 0
then 18, 9
then 24, 9
18, 9
so we take min(data, separator)
that gives 9,9,9,9
now we have the separator cluster
however, if both windows are active during transitions,
you can also capture the higher clusters, which would
allow freq_max to grow
so when you learn e.g.:
freq min = 9
freq max 24
then the learned threshold would be:
(9 + 24) / 2 = 16
and now you know how to separate the tones
fir already suppresses noise so the pulse should
be reliable. so freq/sep only go non-zero when an
actual tone exists
this should now result in being able to sync with
spkmodem encoders with no prior knowledge of the
correct tone frequences. we just use maths.
Signed-off-by: Leah Rowe <leah@libreboot.org>
since we have auto-detection now, we only need to know
that two signals exist, not that they are valid, since
the auto-detection now handles validation and fallback.
Signed-off-by: Leah Rowe <leah@libreboot.org>
a continuation of the previous patch. this waits for
currently one second, before defaulting to the hardcoded
value. otherwise, it tries to use whatever timing it
gets automatically.
this way, the program should now reconfigure its own
timing, without intervention by the user, if the timing
differs from sensible defaults.
this is because spkmodem is implementation-defined;
it's just however coreboot and/or GRUB happen to set
it up, and on the hardware in question.
Signed-off-by: Leah Rowe <leah@libreboot.org>
current logic is hardcoded, as in the original spkmodem-recv.
with this change, small differences are observed and averaged,
then the detection thresholds are adjusted accordingly.
the existing macros serve as a baseline, but real signals
differ. with this change, we therefore account for possible
drift in timings, which can change in real-time; the old
code could possibly get out of sync beccause of that, which
may have resulted in corrupt characters on the screen. this
change therefore should make the output a bit more stable.
the detection window is continually adjusted, so that the
output timings don't drift.
the tolerances are automatically adjusted based on base
timings (see new define in patch)
Signed-off-by: Leah Rowe <leah@libreboot.org>
instead of when it goes above, do it precisely on the
timeout. otherwise, if by sheer chance the signal
pauses and we reset the byte - sure, ok, but it's a
bit tight and we run the risk of advancing another
frame, depending on the timing.
this is a minor edge case, probably rarely ever
triggered in practise.
Signed-off-by: Leah Rowe <leah@libreboot.org>
and with this, i'm now pretty much done modifying grub's
crappy code. this experiment started in 2023 has now
pretty much concluded.
the original GNU code was poorly written, hardcoded
everywhere, and not documented or commented at all.
i had to learn what the code is doing through inference
instead, and i'm pretty sure that these explanations
cover everything. i hope?
maybe the frenchman can explain anything i missed. haha.
Signed-off-by: Leah Rowe <leah@libreboot.org>
yet another optimisation for weaker compilers - but
some modern compilers may not optimise well for this
code either.
this reduces the amount of references to the struct,
which is very expensive (48000 times per second) on
very old CPUs.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the frame[] array is never actually used meaningfully.
that setting of frame[ringpos] on the decode_state is
only set here, but then the value isn't really used at
all. the entire size of the annay is used for sizeof
in print_stats, but then we can just declare that
manually. since we also know that this value never
changes, we can use a global define for the sizeof entry
in print_stats, thereby simplifying operation further
Signed-off-by: Leah Rowe <leah@libreboot.org>
make it easier to read by clearer variable naming.
this change also reduces memory accesses (fewer struct
dereferences - see: struct decoder_state), when using
much weaker/older compilers that don't optimise
properly. this, in the most active part of the code,
which is called.... 48000 times a second. peanuts on
modern CPUs, but on old (early 90s) CPUs it makes a
big difference.
Signed-off-by: Leah Rowe <leah@libreboot.org>
only use the old fallback, or /dev/urandom
/dev/random blocks on some older unix machines,
or in embedded environments that may never
have enough entropy, causing the code to hang.
urandom is most certainly expected to exist on
pretty much anything since the mid 90s.
i could probably re-add the arc4random setup
for BSDs. i'll think about it. gotta do that
portably too.
Signed-off-by: Leah Rowe <leah@libreboot.org>
if someone calls rhex fast enough, the timestamp
may not change. this mitigates that by adding
a counter value to the mix
Signed-off-by: Leah Rowe <leah@libreboot.org>
the last change was good, but this code, again,
has to do these calculations 48,000 times a second.
trivial on new computers. but now try it on a
computer from 1992.
we should try to make this as fast as possible :)
older compilers especially don't optimise these
checks. this patch shifts it to one subtraction and
one unsigned comparison, rather than checking less
than or greater than both. often used in... literally
exactly this type of program.
on a good compiler this will compile to an add, cmp
and conditional jump.
less readable, but the results (set 1 or 0) make it
pretty obvious what it does, after a few seconds.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i turned this into abs() call earlier, but this isn't
obviously readable by some people.
make it absolutely clear what this does. also reduces
use of syscalls.
Signed-off-by: Leah Rowe <leah@libreboot.org>
fread() may return short reads, whereas the current
code assumes either EOF or a full read.
change if to a while. really, it's that simple.
just loop until it's done. i probably b0rked this
myself when refactoring the GNU code.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i treated ftell errors as fatal, but if fttell fails
with ESPIPE, and someone's using -d, the program may
exit immediately, even though there's no problem.
instead, skip printing the offset (basically no debug).
this fixes a bug that i introduced myself, when i forked
this code, because i added that error check; the GNU
code didn't have any check whatsoever.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we currently read small amounts of data with fread,
repeatedly, which is quite taxing on the CPU, on
very old systems.
48khz audio. 48000 calls to fread() per second?
yeah. let's optimise this.
performance now should be roughly O(1) in practise.
this and the other recent changes means no modulo
or division, reduced branching, memory memory roads,
and lots of buffering.
the buffering here is quite conservative, so the human
won't notice any difference. we're cutting the number
of times we call fread by a factor of several thousand,
but you'll still see text scrolling down pretty quick,
with minimal lag.
the old GNU code i forked was terrible at this.
Signed-off-by: Leah Rowe <leah@libreboot.org>
make it clearer about next/old, in the loop. this also
improves performance on older systems (cache the values
first, don't re-calculate)
again, this is GNU code. but you wouldn't know it, in my
current version. i forked this from GRUB several years
ago and started cleaning it for fun.
Signed-off-by: Leah Rowe <leah@libreboot.org>
frame handling, error checks, pulse decoding and
character decoding are all jumbled up. this patch
separates them a bit, making it clearer.
should also help codegen. this tool is dealing with
high bandwidth text, which on slower computers may
be cumbersome. every optimisation counts.
not really relevant on newer systems.
Signed-off-by: Leah Rowe <leah@libreboot.org>
instead of computing next every time, just advance
two indexes. another performance optimisation on
older machines, especially old compilers, because
it reduces the amount of logical branching.
the old code was pretty much just advancing two
indexes in lockstep, when getting the next pulse,
but recalculating one of them based on the other,
each time.
this is yet another hangover from the old GNU code
that i forked three years ago.
Signed-off-by: Leah Rowe <leah@libreboot.org>
it's slow on older compilers/systems that don't optimise.
instead, we branch (cheaper) and just do an above or
equal comparison), resetting appropriately or subtracting.
should yield an actually useful performance gain, on older
systems. a bit theoretical on modern systems, which optimise
well (modern compilers will produce assembly code much like
what the new C code is doing)
Signed-off-by: Leah Rowe <leah@libreboot.org>
a bit pedantic. but that's my intention.
for backwards compatibility with older systems.
this flag means: create the directory. on modern
versions on all systems, it's the default behaviour.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i used a bunch of global variables. that's gone.
added proper externs, including for errno. lots of
old unix systems require this. this version should
be perfectly polished and portable now.
all status is now handled in a struct, making the
code a bit easier to understand, because the variables
now are clearly pertinent to the state of the decoder,
rather than being seemingly random.
some indentation reduced.
also cleaned up ftell/feof usage again. the new code
is a bit more robust when dealing with piped input(which
is literally what this program takes, exclusively)
i started my cleanup of this tool from GNU GRUB in 2023.
i finished it today.
also the Openbsd pledge is more portable now (code made
to compile on pre-pledge openbsd as well)
Signed-off-by: Leah Rowe <leah@libreboot.org>
skip it if there is a valid checksum, to mitigate
erroneous errno state upon exit from run_cmd(),
because we can assume by this point that we
are in fact ready to write at this point.
the check at the end still exists, which will catch
any error set by write, and any error set before
that. this fixes a weird warning on cmd_dump.
Signed-off-by: Leah Rowe <leah@libreboot.org>
reset it in callers instead.
this means that the main function is more generalised.
we know by the time we exit that there is no error.
Signed-off-by: Leah Rowe <leah@libreboot.org>
in the new file i/o functions, my own setting
of errno should be done with set_err. this
avoids clobbering what the real libc set.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i set it to ecanceled before. now i set it more
appropriately, for each type of error.
where a real syscall was called, or my file i/o
functions are used, err() is called with errno
itself as input, to avoid clobbering real errno.
Signed-off-by: Leah Rowe <leah@libreboot.org>
in practise it's ok, but some compilers might complain.
all this change costs is a bit of branching inside a
loop, but compilers will sort that out.
Signed-off-by: Leah Rowe <leah@libreboot.org>
preventative fix, since the values are currently
quite tiny. this new check is the same, but goes
the other way to eliminate overflow.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the old one assumes that ssize_t is signed size_t,
which let's face it, is always true in practise,
but not actually guaranteed!
so now i'm using one that's even more pedantic.
Signed-off-by: Leah Rowe <leah@libreboot.org>
just use errno itself as input to err
if unset, it's set to ECANCELED anyway
i really should rewrite the error handling
to not use errno at some point. it's a bit
unreliable, on some unix systems.
Signed-off-by: Leah Rowe <leah@libreboot.org>
if it resets it on success, that is!
theoretically possible. we must preserve errno.
normally i'm a bit more casual about it, but this
function is replicating libc, so i must be strict
Signed-off-by: Leah Rowe <leah@libreboot.org>
size_t may be unsigned long long, but lu
is for unsigned long. the integer is small
enough that we don't need to worry, so let's
just cast it accordingly (inside err)
Signed-off-by: Leah Rowe <leah@libreboot.org>
always set it. the current logic only sets it if
valid, but invalid doesn't, relying on global
initialisation. this check sets it explicitly.
Signed-off-by: Leah Rowe <leah@libreboot.org>
it currently only does so on success, but errors will
leave the file descriptor corrupted.
reset it accordingly.
Signed-off-by: Leah Rowe <leah@libreboot.org>
currently it returns success, if restoring a
previous offset failed. this leaves descriptor
corrupted when the caller thinks otherwise
return -1 instead, so that the caller can treat
it as an error, relying on whatever lseek had
set for errno
Signed-off-by: Leah Rowe <leah@libreboot.org>
check that it's below len, not above it. that way, it
will now exit if it goes above (which it shouldn't,
but it theoretically could if the code was changed
and there was a regression or subtle edge case)
Signed-off-by: Leah Rowe <leah@libreboot.org>
with the other changes made recently, super old
compilers now work.
yes, i needed to change some specifiers in printf.
typedefs provided for uint, and a define included
X OPEN SOURCE 500. and asserts for integers.
Signed-off-by: Leah Rowe <leah@libreboot.org>
just use /dev/urandom and fall back to /dev/random
this is what i was doing for years. this combined
with other changes, and the new prw() function
for i/o, means portability should be pretty high
now. i will actually start testing nvmutil on old
bsd systems from the 90s later.
Signed-off-by: Leah Rowe <leah@libreboot.org>
in case any stale errors are present.
at this point, we know that the code is likely
safe and that nothing happened, because we quite
obsessively call err() before that point.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the current one assumes two's compliment and no
padding bits. i assert two's compliment earlier
in code, but it doesn't guarantee:
sizeof(ssize_t) == sizeof(size_t)
it's theoretically possible that size_t=64
and ssize_t=32, and then the macro would break.
this new version uses SIZE_MAX instead, without
subtraction, but halves it using a bit shift.
this may still break, but it should work nicely.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i overlooked this when writing. it's comparing
to a length which is size_t, so let's avoid
an unnecessary cast.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we need this to be the case for our code, that char
and uint8_t are 8 bits, and that uint16_t and uint32_t
are 16- and 32-bit.
these asserts protect us in case it's not (it will cause
a compile time error).
Signed-off-by: Leah Rowe <leah@libreboot.org>
in rw_file_exact
otherwise, if length exceeds SSIZE_MAX, we could
hit an overflow
the buffers and lengths we deal with are relatively
small anyway, so this fix is preventative
Signed-off-by: Leah Rowe <leah@libreboot.org>
it now retries infinitely on EINTR, except when the return
of pread is precisely zero, at which point it errs.
this is better than having an arbitrary maximum like before,
and increases robustness on unreliable file systems, e.g.
NFS shares.
Signed-off-by: Leah Rowe <leah@libreboot.org>
only print a message what arg_part is set. this
means that a checksum error message won't be printed
on cat commands.
Signed-off-by: Leah Rowe <leah@libreboot.org>
don't hardcode it per command logically. do it in
the command table instead.
this also fixes a bug where the cat commands did
not set the permissions read-only.
Signed-off-by: Leah Rowe <leah@libreboot.org>
since the cat command can be used to create bad
gbe files, if the checksums don't match. my rule
is that nvmutil must never be used to destroy
data, only correct it (e.g. a file with just one
valid part can have it copied to the other part,
but you can't copy a bad part - and i removed
the "brick" command).
i *did* disable checksum requirements on the
dump command. with this, you can check the nvm
area and it tells you what the correct checksum
could be. then you could just correct it in a
hex editor if you wanted to, quite easily.
the idea is to slow down the act of destroying
or corrupting data as much as possible. someone
wily enough can use a hex editor to patch up some
files just fine.
Signed-off-by: Leah Rowe <leah@libreboot.org>
no build error at the moment, nor would there be if
using clang or gcc, but i imagine some buggy compilers
might complain.
remember: portability. i also want this code to compile
on old, buggy compilers.
logically, this initialisation is redundant.
Signed-off-by: Leah Rowe <leah@libreboot.org>
these take any file size of gbe file: 8KB, 16KB
or 128KB. so does the normal cat.
then you can use cat, cat16 or cat128. these
output to stdout, the corresponding size in KB.
0xFF padding used on the larger files. on the
larger files, the first 4KB of each half is the
GbE parts, and everything else is 0xFF padding.
now you can resize gbe files easily, example:
./nvmutil gbe128.bin > gbe8.bin
yes
Signed-off-by: Leah Rowe <leah@libreboot.org>
it *is* cat. it's catting two GbE parts. so its cat.
(two 4KB areas, plus padding when i add cat16/cat128)
Signed-off-by: Leah Rowe <leah@libreboot.org>
with this, you can read 16KB and 128KB files, and output
them to stdout, but it outputs 8KB
for example:
./nvmutil gbe128.bin > gbe8.bin
now you have a 8KB file
i could probably easily add cat16 and cat128 too.
nvmutil reads two 4KB parts regardless of GbE file
size (one from the first 4KB of each half of the
file), so this was easy to implement.
Signed-off-by: Leah Rowe <leah@libreboot.org>
don't cast unsigned to signed.
no behaviour is changed, but this will prevent some
silly compilers complaining about -Wsign-conversion
Signed-off-by: Leah Rowe <leah@libreboot.org>
i removed this before, but it's good to put it
here defensively, in case i ever mess up
the urandom read function again.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i forgot to handle it in the previous refactor
not really a problem in practise, since the first
read probably succeeds anyway.
Signed-off-by: Leah Rowe <leah@libreboot.org>
also handles possible overflows in read_gbe_file_exact
it removes dead code on both paths: arc4random and
urandom
Signed-off-by: Leah Rowe <leah@libreboot.org>
we rely on uint16_t wrapping, but some platforms may
behave weirdly.
cast as uint32_t and then cast back, on return, with
an explicit mask beforehand.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i plan to release this as a standalone utility at
some point, once it's perfect (on its current
feature set)
Signed-off-by: Leah Rowe <leah@libreboot.org>
i will write a *manpage* at some point. for now, the
documentation on libreboot.org shall suffice.
i'm nearly ready to submit this code to coreboot.
Signed-off-by: Leah Rowe <leah@libreboot.org>
explicitly declare the directory path for the given
file (nvmutil), otherwise it's implementation-defined;
on some systems, /bin/nvmutil means a directory named
nvmutil could then contain nvmutil.
Signed-off-by: Leah Rowe <leah@libreboot.org>
I also needed: #define _POSIX_C_SOURCE 200809L
I use -pedantic with -Wall -Wextra -Werror, which
forces very strict error handling and ISO C; this
means pread and pwrite aren't available.
The define fixes this.
Signed-off-by: Leah Rowe <leah@libreboot.org>
only allow the long form: setmac [MAC]
specifying gbe.bin just shows the help/usage now.
this is a safety feature, so that someone doesn't
accidentally write the gbe file. we want it to be
that the user specifically requested setmac.
setmac with mac address as the 3rd argument is
also disabled. this is done as part of a general
simplification and safety improvement to nvmutil.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this is an extremely dangerous feature, and serves
no purpose to the user.
this change is part of a series of extreme safety
improvements, part of a larger nvmutil audit.
Signed-off-by: Leah Rowe <leah@libreboot.org>
This feature is extremely dangerous, and we should
discourage against its use.
This is part of a series of changes that I've made
to make the code safer. You should only ever run
this on a valid GbE file, and nothing else.
Signed-off-by: Leah Rowe <leah@libreboot.org>
linear, top-down order. re-order the prototypes
also some general cleanup:
argc enums now validated. ifdefs for pledge
and arc4random now use a consistent naming
scheme.
feature change:
the "dump" command now fails if both checksums
are invalid, and won't show anything.
my next commit will disable setchecksum when
both checksums are invalid. this and the other
insane auditing i've done over the last few
days has been part of a major effort to make
nvmutil extremely safe, and robust.
Signed-off-by: Leah Rowe <leah@libreboot.org>
setchecksum and setmac update the checksum.
other commands don't.
this patch unified the logic, handling it
in write_gbe based on command[].chksum_write
Signed-off-by: Leah Rowe <leah@libreboot.org>
only pledge/unveil where available, on versions
that have it. this patch disables it on older
versions, allowing nvmutil to compile.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i previously had this as a speed optimisation, but
removed it because it wouldn't make any real speed
difference, on most modern file systems / kernels.
however, this also has the dual purpose of ensuring
only what was verified gets written, on operations
that only touch the NVM area, since this relies on
checksum verification.
therefore, i have re-added this feature, but under
the new design of nvmutil. it is done policy-based,
instead of having if/else for specific commands.
Signed-off-by: Leah Rowe <leah@libreboot.org>
their functions now only return. not needed anymore.
these commands are still available, but they no longer
need helper functions.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the existing verification is retained, an a few commands.
this is an additional security mechanism. redundancy is
best.
Signed-off-by: Leah Rowe <leah@libreboot.org>
strnlen is not available on some older systems,
so now we provide our own portable version.
this version also aborts on NULL input, unlike
the standard function.
this version also does not permit empty strings.
this version also does not permit unterminated
strings.
Signed-off-by: Leah Rowe <leah@libreboot.org>
arc4random is superior, so using /dev/urandom
would be a mistake. we only use that on linux,
or old/weird unix.
we would also use it on linux, but GNU prohibits
nice things (its implementations are spotty, and
old glibc doesn't have it - before 2022 there is
libbsd, but i'm not importing that).
not that it matters. we're not doing encryption.
i'm just a stickler for technical correctness.
Signed-off-by: Leah Rowe <leah@libreboot.org>
There, we use arc4random_buf which does not directly
access /dev/urandom on BSD; it uses a userspace method
instead, which bypasses this.
This is therefore much more restrictive, which is
exactly the point of unveil(2) and pledge(2); restrict
your program's operation while ensuring that it has what
it needs, to help with debugging and prevent common bugs.
Signed-off-by: Leah Rowe <leah@libreboot.org>
otherwise, it's a pointless computation
i also added a guard to mitigate this, in the
read file function. this should have been there
anyway.
Signed-off-by: Leah Rowe <leah@libreboot.org>
if the enum is messed up, this patch also prevents
that. this is not to catch a runtime error, but
to intentionally trip up a maintainer that screws
up, prompting them to fix their future mistake.
we previously used a pointer directly, without
even checking index/NULL - that too is now covered,
except that we now use an indice for command[] and
execute the command from that, rather than directly
declaring a pointer.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this way, if a user does e.g.
./nvm gbe.bin bullshit
It will say: bullshit
Right now, it just says invalid length. This
means if the user wanted to type e.g.
./nvm gbe.bin copy 0
but they typed:
./nvm gbe.bin coyp 0
Now it will tell them that it's trying
to set the MAC address "coyp".
This is because if an invalid command is given,
it's treated as a MAC address instead. This is
by design, to allow e.g.
./nvm gbe.bin xx:1x:1x:xx:xx:xx
Signed-off-by: Leah Rowe <leah@libreboot.org>
point directly to the command table.
run through an intermediary function to check
bounds, for safety.
this will allow me to then set things like
the invert config directly in that struct.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we need only declare it in the centralised gbe_file_offset
function, which determines whether a write to the gbe file
falls specifically within the 4KB range that is the gbe
part.
it is always half of the gbe file size, and then the first
4KB of each half stores the gbe part.
Signed-off-by: Leah Rowe <leah@libreboot.org>
these variables newrandom and oldrandom are unused on
BSD systems, and their unused status may trigger
warnings on some compilers.
Signed-off-by: Leah Rowe <leah@libreboot.org>
yeah, do the verification manually, don't convert
to size_t. this avoids a bunch of theoretical
bugs that i can't be bothered to explain at 3AM
just trust me bro
Signed-off-by: Leah Rowe <leah@libreboot.org>
preventative fix for later, if the tool is ever expanded
to have a better command syntax, for supporting more than
one file at a time.
Signed-off-by: Leah Rowe <leah@libreboot.org>
send the mac address byte directly to check_mac_separator
functionally identicaly, but cleaner, and uses
multiplication instead of division (faster).
Signed-off-by: Leah Rowe <leah@libreboot.org>
it was put there for another change i haven't done yet,
and probably won't. the program currently just runs once
with one operation, on a given file.
the current defaults are initialised just fine without
this function, so let's remove this bloat for now.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we now handle signedness properly, which is implementation
defined, on char integers where signed/unsigned is not
specified.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the partsize variable is also misleading, because it
refers to the full half of a file, which might be
bigger than 4KB. this variable name is a hangover
from when nvmutil only supported 8KB files.
Signed-off-by: Leah Rowe <leah@libreboot.org>
char can be signed or unsigned, and this is often
implementation-defined. this could result in bad
comparisons, so we should specifically cast it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
This reverts commit 5b120d71e7.
the others are unsigned char. other variables like this one.
better be consistent.
Signed-off-by: Leah Rowe <leah@libreboot.org>
it calls word() anyway, but we should still check it here,
since this is quite a critical function.
the other bound checks are done by word(), which this
function uses to add everything up.
Signed-off-by: Leah Rowe <leah@libreboot.org>
there is 0x20 of different between a and A
so we can just or 0x20 and compare only lowercase.
we can also cast char (which may me signed on some
systems) to unsigned, and then only check whether
it's lower than 10.
this code results in far less branching (in C),
but a good optimising compiler probably wouldn't
have cared about the old version anyway.
it's just nicer C code.
this also means we no longer need to check for
X, only x.
Signed-off-by: Leah Rowe <leah@libreboot.org>
strnlen isn't available on some older unices.
we already know the string will be null-terminated,
because it comes from argv, so runaway reads are
extremely unlikely (read: impossible).
Signed-off-by: Leah Rowe <leah@libreboot.org>
and 1 does the same thing as mod 2, but it's cleaner.
i also now bitshift 3 times instead of times by 8,
which again is clearer in purpose.
i line breaked after h, to make it clear that all of
the next part is being shifted in
Signed-off-by: Leah Rowe <leah@libreboot.org>
sizeof is size_t, so we must act accordingly.
casting it to an int is unacceptable.
this version is also branchless.
Signed-off-by: Leah Rowe <leah@libreboot.org>
fall back to urandom.
also add a /dev/random fallback, for older unices.
with the posix compatibility changes, combined with
this change as above, the code should be portable
now. i expect it to compile on *many* unix systems!
pretty much everything from the last 30 years.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we don't need a whole function. i previously did it
for clarity, but simply setting a variable all in
one line is totally fine.
Signed-off-by: Leah Rowe <leah@libreboot.org>
size_t is generally the size of the address space, so
this is more reliable for our purposes; we're only
working on small buffers, but even so, it's a good
thing to do.
Signed-off-by: Leah Rowe <leah@libreboot.org>
directly handle swapping in word and set_word
in my testing, x86_64 and arm64 compilers actually produce
more efficient code this way. i previously only did a big
swap on the whole buffer on big-endian CPUs, and directly
accessed without swaps on little-endian, as an optimisation.
however, the old code is actually slower than what the
compiler produces, with the new code!
portability is retained with big-endian host CPUs and
little-endian host CPUs.
this also avoids the complication of memcpy and is just
generally extremely reliable by comparison.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we currently never read the 0th byte, so if we need
all 12, and we do when every byte is random, we
read again just to get one byte.
not really a bug, but it is a performance penalty,
so let's fix it!
Signed-off-by: Leah Rowe <leah@libreboot.org>
we want to debug it after the fact; this is now handled,
in the calling functions (unhandled error exceptions).
Signed-off-by: Leah Rowe <leah@libreboot.org>
i don't care. it's only 30 tries.
usleep can fail, setting errno, and it can actually
take longer, depending on the environment. it poisons
errno, and makes debugging harder.
just remove it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we already exit reliably in that function. the current code
is logically correct, but very weak against future changes.
this extra check is essentially redundant, but prevantative
against future changes.
Signed-off-by: Leah Rowe <leah@libreboot.org>
errno shouldn't be set, after reading a file successfully.
if it is, that's a bug. handle it accordingly.
Signed-off-by: Leah Rowe <leah@libreboot.org>
This prevents hogging the CPU in a tight loop,
while waiting for access.
I've also reduced the number of tries to 30, rather
than 200. This is more conservative, while still
being somewhat permissive.
The addition of the usleep delay probably makes
this more reliable than the previous behaviour of
quickly spinning through 200 tries, but without
hogging CPU resources.
I *could* allow this loop to be infinite, but
I regard infinite spin-lock as an error state.
Signed-off-by: Leah Rowe <leah@libreboot.org>
a non-fatal error could have set errno. when we return
from check_read_or_die(), it should be assumed that
all is well.
i don't think this would mask anything important, but
it may be regarded as a preventative bug fix, since
it most likely only prevents false-positives.
Signed-off-by: Leah Rowe <leah@libreboot.org>
Part of the code currently assumes we only work on
the smaller NVM area.
I'm adding some comments to make this clear, for
when and if the code is ever expanded to support
operating on the Extended NVM area (just part the
main 128-byte NVM area).
Signed-off-by: Leah Rowe <leah@libreboot.org>
use of it was preventing more verbose error messages
on exit.
the code is actually cleaner without it, and easier
to read, because of those verbose error messages.
i also added some comments to cmd_swap/copy and did
some other minor/related cleanup elsewhere.
Signed-off-by: Leah Rowe <leah@libreboot.org>
split it out of main. this is good hygiene and it's preparation
for a planned expansion in the future, that allows operation
on multiple files.
Signed-off-by: Leah Rowe <leah@libreboot.org>
currently redundant, but again i might expand this
in the future to allow multiple runs. putting this
here as good practise (currently redundant).
Signed-off-by: Leah Rowe <leah@libreboot.org>
we currently only run the logic once, but i might
expand nvmutil in the future, so that it can
operate on multiple files. this would require
using a different command syntax, e.g. getop-style
syntax.
this is a preventative bug fix, resetting global
state.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we only ever use it once, so it's fine, but future
expansion of this code might trip us up.
this is therefore a preventative bug fix.
Signed-off-by: Leah Rowe <leah@libreboot.org>
size_t can truncate on some platforms. it's best to use
the proper variable type (a cast is insufficient).
Signed-off-by: Leah Rowe <leah@libreboot.org>
strtonum implementations in bsd sometimes have this
variable name. rename it to avoid conflict.
also removed the commentt errno values, since i'm
only ever setting it to valid values, as are the
syscalls that i'm using, so it should be ok.
i'm not writing a stub to check errno. that would
be far beyond the scope of nvmutil.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this is a bit of fault tolerance. a bit bloated too,
but it should make the code more resilient.
we limited the number of retries to 200 retries.
EINTR is when the syscall (read/pread) is interrupted.
we still error out on other conditions; we also still
error out on EINTR if the number of re-tries surpasses
200.
during this re-try loop, if *another* error occurs, we
exit as normal. this is done for both files: the gbe
file, and /dev/urandom.
Signed-off-by: Leah Rowe <leah@libreboot.org>
these functions return ssize_t, so compare explicitly
to that, when using the SIZE_4KB define for example.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we can just use errval as argument to set_err,
because set_err itself now properly handles
errno, ensuring that is is never set to zero.
Signed-off-by: Leah Rowe <leah@libreboot.org>
word/set_word are only meant to operate on the nvm
area (128 bytes), but the current check is against
the entire 4KB block.
swap() only handles the nvm area, as per the design
of nvmutil.
this patch makes the boundary check truer to my real
intent, guarding against future logical errors.
Signed-off-by: Leah Rowe <leah@libreboot.org>
alignment isn't an issue, but aliasing between uintX_t
types in C means that this code may fail on some weird
systems.
using memcpy here is advisable.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we don't need it. what follows is a call to open(), which
would fail anyway if the path is a directory; further, this
removes a theoretical race condition in the program, and
makes open() happen sooner, making it more likely that we
get the file first, before another program can take it.
checking whether /dev/urandom is a directory is the height
of absurdity.
Signed-off-by: Leah Rowe <leah@libreboot.org>
n + 1 is the same as saying sizeof(rnum) in this case.
we should be clear about that, in code. n is irrelevant
here, since it is only an index for the return value.
Signed-off-by: Leah Rowe <leah@libreboot.org>
checking against -1 is incorrect, because we specifically want
to ensure that it always read the number of bytes defined by
the size of rnum.
this still covers the case where the return value is -1, and
therefore makes the error handling much stricter.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the way err works here now is very different than
the bsd one. here, we ALWAYS exit with EXIT_FAILURE,
and we call set_err with, as argument, the first
argument given to err.
this then sets errno, but the exit value is always
consistent.
that's what happens when i control err(). i make it
even better. the original bsd one is too conservative.
Signed-off-by: Leah Rowe <leah@libreboot.org>
and getprogname, written as getnvmprogname
this removes a dependency on err.h, which is non-standard.
the remaining code is posix-compliant, or ifdef'd where
use of openbsd pledge is concerned - someone could theoretically
define __OpenBSD__ that isn't and OpenBSD base maintainer, and
then use nvmutil in it, but i so don't care about that evermore
hypothetical individual.
Signed-off-by: Leah Rowe <leah@libreboot.org>
by handling close() in main, we can reduce the
indendation in write_gbe and generally make it
much easier to read.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the swap function reverses the byte order in memory, of
a loaded GbE after after reading it, or before writing
it. this is required (as detected) on big-endian CPUs,
because GbE files store bytes in little-endian order.
Signed-off-by: Leah Rowe <leah@libreboot.org>
set it after the argc check
i don't like initialising a const after
declaration, but it compiles, and it keeps
with the style used in the rest of the code.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i declare it, using the 3rd argument, which might
not be available if only the file name is declared.
this fixes that.
Signed-off-by: Leah Rowe <leah@libreboot.org>
these macros serve no purpose except to obfuscate the
code. it's actually cleaner just to refer directly to
argv, and it reduces the chance of contamination later
upon re-factoring.
Signed-off-by: Leah Rowe <leah@libreboot.org>
and op to ops
typedefs not part of any base system e.g. openbsd
base system, or e.g. the libc, should not have _t
in them.
this is a stylistic change, and does not alter any
actual program behaviour.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the second half of the function is mostly checking
arguments, and has the hack to set cmd to cmd_setmac
if cmd is unset (no command specified) but argc is
above two.
stylistically, this is more consistent.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this fixes the bug where if you specify an invalid command
such as:
./nvm gbe brick 9
part 9 doesn't exist, but fname isn't yet set, here.
same thing applys when running those pledge commands on
openbsd.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this is separate from other function calls. err_if
is used as though it was an if, where we always add
a space. it's just a quirk of my coding style.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this way, the correct part number is printed when an invalid
part is being operated on, in cmd copy or swap.
Signed-off-by: Leah Rowe <leah@libreboot.org>
instead, use a single integer, set to 1 if using
these commands (otherwise set to 0) used as an XOR
mask.
use this to invert where data gets read. one quirk
with this is that if a copy operation is performed
from a part with a bad checksum, it's already done
in advance, in memory, but then the check on the
checksum in cmd_copy is now checking the other part,
which will be all zeroes, so i invert that too; this
means now when running cmd_copy, it'll complain about
an invalid part, but the part number is inverted.
it's a small price to pay, because this restores the
previous performance optimisations but without being
as unsafe.
this is also true when doing the swap.
Signed-off-by: Leah Rowe <leah@libreboot.org>
lots of block devices use 4KB block size. it makes
sense to have this optimisation here.
i previously removed it, along with the one that
only reads the NVM area - that one is still gone,
because it was largely pointless.
because of this modification returning, i also
re-introduced the check in setWord against
nvmPartModified - otherwise, for example, running
cmd brick 0 would brick part 0 but then write
all zeroes to part 1.
Signed-off-by: Leah Rowe <leah@libreboot.org>
those lines at the end are a hangover from the old opendir-
based implementation.
i also made the output more verbose in that first error
check.
Signed-off-by: Leah Rowe <leah@libreboot.org>
opendir allocates resources and causes a bunch of other
error conditions which we need to catch.
use of stat is more efficient here.
Signed-off-by: Leah Rowe <leah@libreboot.org>
it still had some leftovers from the old macro-style
implementation. it still compiled, but this patch
fixes the function properly.
Signed-off-by: Leah Rowe <leah@libreboot.org>
irrelevant for most users, who are on little endian
anyway, but i broke the swap function on big endian
systems. this fixes it.
the new function uses an intermediate variable instead
of xor swapping, but i accidentally left some relics of
of the old xor swaps in place. this fixes that.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this, in conjunction with the centralised exit scheme now
used by nvmutil, means that we have portable exit status.
notwithstanding the use of non-portable unix functions, and
especially the use of non-standard err.c (which GNU and BSD
libc implementations all have anyway, as does musl).
this code should now run on essentially any computer with
Linux or BSD on it.
Signed-off-by: Leah Rowe <leah@libreboot.org>
exit with 0 or 1, as is proper.
errno is an int, but the return value on a shell
can be e.g. byte, and depending how that number (errno)
is valued, could overflow and cause a zero exit, where
you want a non-zero exit.
the code has been changed, in such a way to maintain
current behaviour (don't change errno), except that when
errno is set upon exit, the exit value is now one.
Signed-off-by: Leah Rowe <leah@libreboot.org>
pointless optimisation. we know that when a user requests an
operation that would write, it will probably result in a change.
therefore, this change is the real optimisation. to avoid
writing the same half of a file twice, when using cmd_copy,
we check (in writeGbe) whether gbe part 0 and 1 are the same;
if they are, then we only loop once. this is important, because
otherwise we would call swap() twice.
this means that the optimisations in cmd_copy and cmd_swap must
be removed. the point of this and other changes is to improve
memory safety in nvmutil, so frivolous use of pointers has to go.
Signed-off-by: Leah Rowe <leah@libreboot.org>
modern file systems work in 4KB blocks. reading only
a small part of it doesn't really make much difference
in terms of performance.
simplify the code instead.
Signed-off-by: Leah Rowe <leah@libreboot.org>
modern malloc implementations make the optimisation here
pretty pointless.
modern computers make this modification pointless.
i'm not planning to run nvmutil on a VAX. openbsd removed
support for it ages ago. 8KB fixed buffer is fine.
Signed-off-by: Leah Rowe <leah@libreboot.org>
it doesn't save any time on modern systems, and it's just
confusing for some people to read. i mean, i understand it
instinctively, but normal people do it with a swap variable.
Signed-off-by: Leah Rowe <leah@libreboot.org>
we always want unveil/pledge calls to be in main, when
possible, so that they are more transparent and easier
to understand when re-factoring, because it's extremely
important that these syscalls be done correctly.
main is small enough now, from other re-factoring changes,
that i'm happy to have this back in main now.
Signed-off-by: Leah Rowe <leah@libreboot.org>
the current check is too liberal. make it sticter.
the issue is that the previous check did not take
into account that it's a check on a uint16_t array,
against nf which refers to a number of bytes.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this was the other complication with doing it as a macro.
for something this fundamental, we really want to ensure
that every access is safe.
Signed-off-by: Leah Rowe <leah@libreboot.org>
merge the urandom handling back into this function.
it's called immediately after in main anyway, so we
may as well. this reduces the size of main.
Signed-off-by: Leah Rowe <leah@libreboot.org>
in the given call, we then do an equivalent call
immediately after that is the same, but without
unveil, so we'll just defer to that.
this changes no behaviour.
Signed-off-by: Leah Rowe <leah@libreboot.org>
in general, we should ensure that the pledge calls only happen
inside main. this means we can more easily see them, in future
re-factoring.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this will enable hardening of the pledge syscalls.
it also means that the program will error out much
earlier, when an invalid command is given, rather
than opening a bunch of files first, and it will
do so under reduced privilege already, notwithstanding
the further pledge/unveil hardening that is planned.
Signed-off-by: Leah Rowe <leah@libreboot.org>
urandom in main. this is because i'm going to further
harden the use of pledge and unveil in a future patch,
and this is a prerequisite.
Signed-off-by: Leah Rowe <leah@libreboot.org>
also for other variants
i removed it because it was reported broken. it's not.
the removal was always temporary, pending further testing.
next time, i will be more sceptical.
everything works fine.
Signed-off-by: Leah Rowe <leah@libreboot.org>
do it in the macro. this way, if a given error is
present, it's not overridden. this enables easier
debugging.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i renamed filename to fname, so that certain lines would
still fit within 80 characters without introducing a new
line break.
Signed-off-by: Leah Rowe <leah@libreboot.org>
handle it in a separate function, for clarity.
the main function just checks each part whether it
changed, and then passes control to writeGbe_part.
Signed-off-by: Leah Rowe <leah@libreboot.org>
use the ERR macro instead, so that an existing value
will not be overridden. this is useful for debugging.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this has to do with memory allocation, not actual reading
of the gbe file into memory. split it up, for clarity.
Signed-off-by: Leah Rowe <leah@libreboot.org>
when nf and nr/nw are not the same, we know there
is an error condition, so defer to the following err()
call, but use ERR() there instead of hardcoding use
of ECANCELED.
this actually improves the error handling, by being
more verbose, while reducing the amount of logic.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this is a special mode that skips FPTR checks, which is
needed on the topton x2e_n150
we currently set this, when MEclean="n", but we may want to
skip cleaning while still checking FPTR on some boards (in
a future lbmk revision)
Signed-off-by: Leah Rowe <leah@libreboot.org>
it is currently only initialised inside case
conditions. this is fine on most shells, but
some of them can be a bit buggy here.
initialise it empty and then override.
Signed-off-by: Leah Rowe <leah@libreboot.org>
this is a hangover from an earlier work, where we
had some issues prior to merging.
as it is, the port is ready for a future release.
Signed-off-by: Leah Rowe <leah@libreboot.org>
Courtesy of Kat Inskip who ported this board.
Headphone output doesn't work at the moment, due to incorrect verb.
Intel VBT is also wrong. Both are taken from another board.
This will be amended later with the correct verb and VBT.
Signed-off-by: Leah Rowe <leah@libreboot.org>
latest coreboot rev as of literally today
this is in preparation for a thinkpad x270 port
using a WIP patch that was contributed
Signed-off-by: Leah Rowe <leah@libreboot.org>
this is the program I recently wrote, that generated
the submodule entries for the GRUB PO file fix
this utility is for reference only. i'll probably do
away with the fix at some point, replacing it with
my own git-based submodule repository, containing the
PO files. this would make things easier, and then
that repository would contain the utility instead.
i'm just putting this in lbmk for now, so that we
have it somewhere.
Signed-off-by: Leah Rowe <leah@libreboot.org>
Using the same ME image as the 3050 Micro. This fixes the lack of a
backup download URL for the ME and speeds up boot time, since MRC
caching is working with ME (unlike SPS).
Unfortunately, since the MFS partition must be preserved, this does
mean we need a larger ME region than with me_cleaned SPS.
the lack of redundancy in Intel ME downloading is a current
release blocker with this board, so set it to release=n for
now.
it is quite possible to use deguard on this board, which does
have redundant downloading when used with lbmk.
although the board doesn't have bootguard, it is still possible
to use deguard. you can configure the generic ME image that it
fetches, and reconfigure it for each machine.
i've asked ron to look into this, on their test board.
Signed-off-by: Leah Rowe <leah@libreboot.org>
i don't like hyphens in file names, because of how lbmk
has historically handled directories and files in the past;
i've removed a lot of eval statements, to the extent that
it's no longer likely to be a problem (it's barely used now),
but i previously had a problem with using hyphens in config
names.
this design flaw (in lbmk) was fixed ages ago, but i still
maintain this policy. since that time, i use hyphens only.
Signed-off-by: Leah Rowe <leah@libreboot.org>
Surprisingly, SeaBIOS VGA output works (coreboot documentation says it
doesn't).
I'm using a static CMOS option table currently (like most other boards
supported in libreboot), but maybe it would be better to switch to the
CBFS file option table. The default option table enables
hyperthreading, overriding the compile-time setting.
I'm also using a ME/SPS image extracted from the official BIOS update
for this board. Unfortunately, https://www.supermicro.com/Bios/* is
excluded from crawlers in robots.txt so it's not in archive.org, so I
haven't been able to find a backup download URL. I also needed to set
the user-agent for fetching the update to "curl/8.6.0" because the
default user-agent override used by lbmk resulted in a 403 error.
deguard is not required (there's no bootguard on this board).
SPS does not implement CPU replacement detection which means that the
MRC cache does not work and RAM training needs to happen on every
boot. To avoid this it may be possible to run ME instead of SPS on
this board, but I tried both the ME image used on the OptiPlex 3050
Micro in libreboot and one from the ASRock C236 WSI and they both hung
at "[INFO ] POST: 0x92" (POSTCODE_FSP_MEMORY_INIT).
The memtest86+ build included with libreboot doesn't work with USB
keyboards and this board doesn't have a PS/2 port, which is annoying.
The files it downloads are not versioned, and they could
change any time. GRUB has no way to deterministically grab
these.
I've removed GRUB's local for grabbing these, instead
mirroring them myself and checking hashes; no hashes seem
to have been provided by the upstream at Translation Project,
so I just used the hashes I had on the files it had, when
I downloaded them.
From now on, I can just re-download these and re-calculate
the hashes as desired, over time, when updating GRUB revisions.
Signed-off-by: Leah Rowe <leah@libreboot.org>
2026-02-01 22:09:33 +01:00
417 changed files with 23212 additions and 387 deletions
From f5f73c2539e05cf85bf5eec795e4f91da50838ba Mon Sep 17 00:00:00 2001
From: Kat Inskip <kat@inskip.me>
Date: Tue, 17 Feb 2026 16:18:15 -0800
Subject: [PATCH 48/48] mb/lenovo/sklkbl: Add Lenovo Thinkpad X270 as a variant
This machine is somewhat dissimilar from the X280 in the PCIe allocations in the overridetree. It also lacks soldered RAM, having a single SODIMM slot.
This port was based upon the work done by Johann C Rode for the X280 and the VBT and hda verbs were obtained from that work, not obtained separately. GPIO ports and PCI-e allocations have been checked against schematics after editing.
Functionality has been validated on a ThinkPad X270 with machine type model 20HMS2WU03 with 16GB onboard RAM and i5-7300U CPU. The laptop has been tested running libreboot, booting Guix via GRUB payload. A check of the hardware shows no issues (video, wifi, wired ethernet, reboot, sleep, NVMe).
An untested variety allowing for a Skylake CPU (for 20K5 and 20K6) has been included.
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Leah Rowe
Leah Rowe
Ron Nazarov
Todd Baker
AlguienSasaki
duchy
Kat Inskip
Niel Nielsen