On Tue, 4 Mar 2025 at 12:49, Gerd Hoffmann <kra...@redhat.com> wrote:
>
> Define qapi schema for the uefi variable store state.
>
> Use it and the generated visitor helper functions to store persistent
> (EFI_VARIABLE_NON_VOLATILE) variables in JSON format on disk.
>
> Acked-by: Markus Armbruster <arm...@redhat.com>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> Message-ID: <20250225163031.1409078-15-kra...@redhat.com>
> [ incremental fix squashed in ]
> Message-ID: <pji24p6oag7cn2rovus7rquo7q2c6tokuquobfro2sqorky7vu@tk7cxud6jw7f>
Hi; Coverity points out some problems in this code
(CID 1593154, 1593157):
> +void uefi_vars_json_load(uefi_vars_state *uv, Error **errp)
> +{
> + UefiVarStore *vs;
> + QObject *qobj;
> + Visitor *v;
> + char *str;
> + size_t len;
> + int rc;
> +
> + if (uv->jsonfd == -1) {
> + return;
> + }
> +
> + len = lseek(uv->jsonfd, 0, SEEK_END);
lseek() can return -1 on error, but we don't check that
here, so we can proceed forward with a negative length...
> + if (len == 0) {
> + return;
> + }
> +
> + str = g_malloc(len + 1);
...so we might try to malloc(0) here...
> + lseek(uv->jsonfd, 0, SEEK_SET);
> + rc = read(uv->jsonfd, str, len);
...and read() will treat -1 as a very large positive
value, overrunning the buffer.
> + if (rc != len) {
> + warn_report("%s: read error", __func__);
If the read failed to read all the data, we need to
bail out now, not proceed to pass the uninitialized
buffer to qobject_from_json().
Coverity doesn't notice it, but the code in
uefi_vars_json_save() is also not handling errors
from lseek() etc correctly.
Do we absolutely need to be operating on a continuously open
filedescriptor here rather than a filename? If we could make
these functions use uv->jsonfile each time then we could use
the glib functions g_file_get_contents() and
g_file_set_contents(), which get all the pesky details of the
error handling right for you.
thanks
-- PMM
