From patchwork Fri Aug 2 11:49:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Uwe Schuster X-Patchwork-Id: 1141116 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=googlegroups.com (client-ip=2607:f8b0:4864:20::b3b; helo=mail-yb1-xb3b.google.com; envelope-from=swupdate+bncbcvk3t7xzipbbwogsdvakgqenarxszy@googlegroups.com; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=googlegroups.com header.i=@googlegroups.com header.b="L+MEO1yp"; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="V00Duzfr"; dkim-atps=neutral Received: from mail-yb1-xb3b.google.com (mail-yb1-xb3b.google.com [IPv6:2607:f8b0:4864:20::b3b]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 460QSZ6SR6z9sBF for ; Fri, 2 Aug 2019 21:49:49 +1000 (AEST) Received: by mail-yb1-xb3b.google.com with SMTP id w6sf56976193ybo.2 for ; Fri, 02 Aug 2019 04:49:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlegroups.com; s=20161025; h=sender:date:from:to:message-id:subject:mime-version :x-original-sender:precedence:mailing-list:list-id:list-post :list-help:list-archive:list-subscribe:list-unsubscribe; bh=E4PeJTHxS3DepUlsQK5wNo68Nr2HbmmcWXtsgYFaozs=; b=L+MEO1ypw3Z8N6Iroi5nq2kkB9tEgC+zstHPHxsIAcpd/Qn4xtY9SPZthYZoPFvAYI 6/pRhDyjnXwHxaQ4XpLe+VprgrQnke7SzkzLPxv+UoZJMuaLtBPbIz3ELi8cU3/4uyHA BrscHHPkpMwvMHDYOcKb0jhZ23SQQJqL/dLHniSKmRlePdEgrQNgSEkbeBtb+nNEAncn mQyyNEN/PVFnhH/GdJx+uijaVCZvk6SlINs8MIJFS3Vr3XihlS+2pgpuSZt7BRBQJ3j8 Qg11cLyNfhKI6DEeJmr9U7D5s8Mdc9ebYS1LmVEKKKLwqfdC5lXr01glSP/GZfVsGmQw f9bQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:message-id:subject:mime-version:x-original-sender :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-subscribe:list-unsubscribe; bh=E4PeJTHxS3DepUlsQK5wNo68Nr2HbmmcWXtsgYFaozs=; b=V00DuzfrHUoWMaPJ0U30pfpb9KqhL2FJlsYB/C8/ZOCub6reDOi0KX/ZV9HftFHt5d 17GZJfEpl18QYFALxEWgt/wB7d/ClZiECa4hYAe1pYl+iQPTE3GWnJnZEMVCg58OA1EQ NA0MITr9zmryrYoIdxsjEtkneRgz1SVi72GiPJSlOC28rvS4Cw+tYgWV1MmjFdcUc7/3 7LMMH0TWMZw+DYA8lctODejIV1xndjYinbJZK/hGK0Mx81FSVzY852ib6vOIoJy9jZNW wvRKwJw99boRlVT/Xu+lNToomvmjsAe6aZ2fZWFibmR4Y03fGWscKzW07z8RfO0vC9bT 782A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=sender:x-gm-message-state:date:from:to:message-id:subject :mime-version:x-original-sender:precedence:mailing-list:list-id :x-spam-checked-in-group:list-post:list-help:list-archive :list-subscribe:list-unsubscribe; bh=E4PeJTHxS3DepUlsQK5wNo68Nr2HbmmcWXtsgYFaozs=; b=RPr4h/4VrVqSFXY9KzUxzbFRJacMSY5+JJEHLxlzmmBjpB89vYGaIi3dF+QKkCEtc3 kw+WAGUHaxIUqnt+hVqya8P41gVmjNVa1JlWYkyzB/23ZuTA8BzNn2/vKl4d1FtLM2v+ 6q6YoRPoYQTqCOnAT4/PrO9gne6jcsu2D6pnyOQQAw51xwSSRNTe7oT31H2/NUq+uKCf ll+dbFvcLrTzZiZqdG8ntmEuuc04OgUmFKTQmjgf6mGjWS8DugWvXcqbkgNJzAzgGvsk 5Km4usTLi/wy+KLzvmNC4lbm395ajoqkDyngAUb2Bw1AhdBQV7sxkvKTrzHMy1xqchfB x/fg== Sender: swupdate@googlegroups.com X-Gm-Message-State: APjAAAVL0C3AMBFNKNRs4/QmohhcwgqMRPzWxZLAfpLIvpSBKlEwGtxu qtb3vsjGOs1KDT6pvmLuNlA= X-Google-Smtp-Source: APXvYqwGXe3Z8GMuWUi9KrgMhMOAd/DkC4TLsDx0c9Zp47GaJFnPdEp1WBDGTHSVz3D+KpmZpJYmiw== X-Received: by 2002:a81:9b8e:: with SMTP id s136mr86487553ywg.114.1564746585996; Fri, 02 Aug 2019 04:49:45 -0700 (PDT) X-BeenThere: swupdate@googlegroups.com Received: by 2002:a0d:d994:: with SMTP id b142ls12798245ywe.2.gmail; Fri, 02 Aug 2019 04:49:45 -0700 (PDT) X-Received: by 2002:a81:1dc5:: with SMTP id d188mr75335520ywd.185.1564746584972; Fri, 02 Aug 2019 04:49:44 -0700 (PDT) Date: Fri, 2 Aug 2019 04:49:44 -0700 (PDT) From: Uwe Schuster To: swupdate Message-Id: <3a0d972a-f06b-40ca-8cae-fa52af360afb@googlegroups.com> Subject: [swupdate] [libubootenv] [PATCH] uboot_env: Fix handling of .flags variable when it contains variables which are not created yet MIME-Version: 1.0 X-Original-Sender: uwe.schuster68@gmail.com Precedence: list Mailing-list: list swupdate@googlegroups.com; contact swupdate+owners@googlegroups.com List-ID: X-Spam-Checked-In-Group: swupdate@googlegroups.com X-Google-Group-Id: 605343134186 List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , This tries to fix the problem with variable access or type flags which are part of the .flags variable but do not exist as a env variable as reported by me in https://groups.google.com/d/msg/swupdate/ShdD10dl7Pc/ULUYSpFDCAAJ: 1. The write once (o) access atrribute flag is not properly handled if the > variable referenced by the .flags value string does not exist yet (as it > would be the case with write once variables before they are first written > or set). Instead this variables flags gets removed from .flags value and > attributes are lost. > This behavior also applies to all other variables which do not exist yet, > but in case of write once variables I'd consider it a normal use case. > I had to choose an implementation and I chose to put such uncreated variables into the metadata database as normal variables but with NULL value assigned. The patch also fixes a problem with the existing parsing function. It failed to handle .flags value strings with more then one variable (comma separated). The only known problem what remains is with fw_printenv function. It does currently not print out the .flags variable (as normal u-boot would). To fix that more changes to the printenv implementation will be required (reconcat .flags from internal list structure) so I decided to omit it. --- src/fw_printenv.c | 16 +++-- src/uboot_env.c | 176 ++++++++++++++++++++++++++++++---------------- 2 files changed, 125 insertions(+), 67 deletions(-) buf += snprintf(buf, size, ".flags="); @@ -780,10 +789,15 @@ int libuboot_env_store(struct uboot_ctx *ctx) attr_tostring(entry->type), access_tostring(entry->access)); } + first = false; } } *buf++ = '\0'; +#if !defined(NDEBUG) + fprintf(stdout, "Saved environment FLAGS %s\n", flagsbuf); +#endif + if (ctx->redundant) { unsigned char flags = ctx->envdevs[ctx->current].flags; switch(ctx->envdevs[ctx->current].flagstype) { @@ -817,18 +831,19 @@ int libuboot_env_store(struct uboot_ctx *ctx) return ret; } -static void libuboot_parseflags(struct uboot_ctx *ctx, char* flagsvar) +static void libuboot_parseflags(struct uboot_ctx *ctx, const char* flags) { char *pvar; char *pval; struct var_entry *entry; + char *flagsvar = strdup(flags); #if !defined(NDEBUG) - fprintf(stdout, "Environment FLAGS %s\n", flagsvar); + fprintf(stdout, "Parsed environment FLAGS %s\n", flags); #endif pvar = flagsvar; - while (*pvar && (pvar - flagsvar) < strlen(flagsvar)) { + while (pvar && ((pvar - flagsvar) < strlen(flags))) { char *pnext; pval = strchr(pvar, ':'); if (!pval) @@ -841,47 +856,56 @@ static void libuboot_parseflags(struct uboot_ctx *ctx, char* flagsvar) else *pnext++ = '\0'; +#if !defined(NDEBUG) + fprintf(stdout, "Parsed var %s:%s\n", pvar, pval); +#endif entry = __libuboot_get_env(&ctx->varlist, pvar); - if (entry) { - for (int i = 0; i < strlen(pval); i++) { - switch (pval[i]) { - case 's': - entry->type = TYPE_ATTR_STRING; - break; - case 'd': - entry->type = TYPE_ATTR_DECIMAL; - break; - case 'x': - entry->type = TYPE_ATTR_HEX; - break; - case 'b': - entry->type = TYPE_ATTR_BOOL; - break; - case 'i': - entry->type = TYPE_ATTR_IP; - break; - case 'm': - entry->type = TYPE_ATTR_MAC; - break; - case 'a': - entry->access = ACCESS_ATTR_ANY; - break; - case 'r': - entry->access = ACCESS_ATTR_READ_ONLY; - break; - case 'o': - entry->access = ACCESS_ATTR_WRITE_ONCE; - break; - case 'c': - entry->access = ACCESS_ATTR_CHANGE_DEFAULT; - break; - default: /* ignore it */ - break; - } + if (!entry) { + /* create non-valued entry to conserve its flags */ + libuboot_set_env(ctx, pvar, NULL); + entry = __libuboot_get_env(&ctx->varlist, pvar); + } + + for (int i = 0; i < strlen(pval); i++) { + switch (pval[i]) { + case 's': + entry->type = TYPE_ATTR_STRING; + break; + case 'd': + entry->type = TYPE_ATTR_DECIMAL; + break; + case 'x': + entry->type = TYPE_ATTR_HEX; + break; + case 'b': + entry->type = TYPE_ATTR_BOOL; + break; + case 'i': + entry->type = TYPE_ATTR_IP; + break; + case 'm': + entry->type = TYPE_ATTR_MAC; + break; + case 'a': + entry->access = ACCESS_ATTR_ANY; + break; + case 'r': + entry->access = ACCESS_ATTR_READ_ONLY; + break; + case 'o': + entry->access = ACCESS_ATTR_WRITE_ONCE; + break; + case 'c': + entry->access = ACCESS_ATTR_CHANGE_DEFAULT; + break; + default: /* ignore it */ + break; } } + pvar = pnext; } + free(flagsvar); } static int libuboot_load(struct uboot_ctx *ctx) @@ -1061,10 +1085,10 @@ int libuboot_load_file(struct uboot_ctx *ctx, const char *filename) name = buf; - if (!strcmp(name, ".flags")) - flagsvar = strdup(value); + if (!strcmp(name, ".flags")) + flagsvar = strdup(value); else - libuboot_set_env(ctx, name, value); + libuboot_set_env(ctx, name, value); } /* @@ -1158,12 +1182,19 @@ static bool libuboot_validate_flags(struct var_entry *entry, const char *value) ok_access = true; break; case ACCESS_ATTR_READ_ONLY: - case ACCESS_ATTR_WRITE_ONCE: ok_access = false; break; + case ACCESS_ATTR_WRITE_ONCE: + if (entry->value) { + ok_access = false; + } + break; case ACCESS_ATTR_CHANGE_DEFAULT: break; } +#if !defined(NDEBUG) + fprintf(stdout, "Access flag validation %s\n", ok_access ? "success." : "failed."); +#endif if (!ok_access) return false; @@ -1201,6 +1232,10 @@ static bool libuboot_validate_flags(struct var_entry *entry, const char *value) ok_type = false; break; } +#if !defined(NDEBUG) + fprintf(stdout, "Type flag validation %s\n", ok_type ? "success." : "failed."); +#endif + return ok_type; } @@ -1209,13 +1244,30 @@ int libuboot_set_env(struct uboot_ctx *ctx, const char *varname, const char *val struct var_entry *entry, *elm, *lastentry; struct vars *envs = &ctx->varlist; + /* update all .flags attributes in list with new value */ + if (!strcmp(varname, ".flags")) { + LIST_FOREACH(entry, &ctx->varlist, next) { + entry->type = TYPE_ATTR_STRING; + entry->access = ACCESS_ATTR_ANY; + } + if (value) + libuboot_parseflags(ctx, strdup(value)); + + return 0; + } + entry = __libuboot_get_env(envs, varname); if (entry) { if (libuboot_validate_flags(entry, value)) { - if (!value) { - free_var_entry(envs, entry); - } else { - free(entry->value); + if (entry->value) { + if (!value) { + free_var_entry(envs, entry); + } else { + free(entry->value); + entry->value = strdup(value); + } + } + else if (value) { entry->value = strdup(value); } return 0; @@ -1224,9 +1276,6 @@ int libuboot_set_env(struct uboot_ctx *ctx, const char *varname, const char *val } } - if (!value) - return 0; - entry = (struct var_entry *)calloc(1, sizeof(*entry)); if (!entry) return -ENOMEM; @@ -1235,11 +1284,16 @@ int libuboot_set_env(struct uboot_ctx *ctx, const char *varname, const char *val free(entry); return -ENOMEM; } - entry->value = strdup(value); - if (!entry->value) { - free(entry->name); - free(entry); - return -ENOMEM; + if (!value) { + entry->value = NULL; + } + else { + entry->value = strdup(value); + if (!entry->value) { + free(entry->name); + free(entry); + return -ENOMEM; + } } lastentry = NULL; diff --git a/src/fw_printenv.c b/src/fw_printenv.c index 18887f9..8fd1f55 100644 --- a/src/fw_printenv.c +++ b/src/fw_printenv.c @@ -137,17 +137,21 @@ int main (int argc, char **argv) { if (!argc) { tmp = NULL; while ((tmp = libuboot_iterator(ctx, tmp)) != NULL) { - name = libuboot_getname(tmp); value = libuboot_getvalue(tmp); - fprintf(stdout, "%s=%s\n", name, value); + if (value) { + name = libuboot_getname(tmp); + fprintf(stdout, "%s=%s\n", name, value); + } } } else { for (i = 0; i < argc; i++) { value = libuboot_get_env(ctx, argv[i]); - if (noheader) - fprintf(stdout, "%s\n", value ? value : ""); - else - fprintf(stdout, "%s=%s\n", argv[i], value ? value : ""); + if (value) { + if (noheader) + fprintf(stdout, "%s\n", value ? value : ""); + else + fprintf(stdout, "%s=%s\n", argv[i], value ? value : ""); + } } } } else { /* setenv branch */ diff --git a/src/uboot_env.c b/src/uboot_env.c index 09d210d..bf60635 100644 --- a/src/uboot_env.c +++ b/src/uboot_env.c @@ -735,6 +735,9 @@ int libuboot_env_store(struct uboot_ctx *ctx) uint8_t offsetdata; int ret; int copy; +#if !defined(NDEBUG) + char * flagsbuf; +#endif /* * Allocate the bigger of the case @@ -752,21 +755,27 @@ int libuboot_env_store(struct uboot_ctx *ctx) buf = data; LIST_FOREACH(entry, &ctx->varlist, next) { - size = (ctx->size - offsetdata) - (buf - data); - if ((strlen(entry->name) + strlen(entry->value) + 2) > size) - return -ENOMEM; + if (entry->value) { + /* only save variables which have a value (do exist yet) .. */ + size = (ctx->size - offsetdata) - (buf - data); + if ((strlen(entry->name) + strlen(entry->value) + 2) > size) + return -ENOMEM; + buf += snprintf(buf, size, "%s=%s", entry->name, entry->value); + buf++; + } + /* .. but include non yet existing vars into .flags string */ if (entry->type || entry->access) saveflags = true; - - buf += snprintf(buf, size, "%s=%s", entry->name, entry->value); - buf++; } /* * Now save the .flags */ if (saveflags) { +#if !defined(NDEBUG) + flagsbuf = buf; +#endif bool first = true; size = (ctx->size - offsetdata) - (buf - data);