From patchwork Thu Dec 10 21:52:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sava Jakovljev X-Patchwork-Id: 1414578 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=googlegroups.com (client-ip=2a00:1450:4864:20::33f; helo=mail-wm1-x33f.google.com; envelope-from=swupdate+bncbaabbjnrzl7akgqevsgjo3a@googlegroups.com; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=teufel.de Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=googlegroups.com header.i=@googlegroups.com header.a=rsa-sha256 header.s=20161025 header.b=g5i80fnF; dkim-atps=neutral Received: from mail-wm1-x33f.google.com (mail-wm1-x33f.google.com [IPv6:2a00:1450:4864:20::33f]) (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 4CsSMG5M6bz9sSf for ; Fri, 11 Dec 2020 08:52:42 +1100 (AEDT) Received: by mail-wm1-x33f.google.com with SMTP id z12sf2540535wmf.9 for ; Thu, 10 Dec 2020 13:52:42 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1607637158; cv=pass; d=google.com; s=arc-20160816; b=JjDc+pcJjqIvuRhcttImkBXqqyuo/EUo63VaUalEbIilqbMHmlT07icChytC+oORzC otTdGBlbdFh7enOzAlW83vR8EaG+gQzYXMxf2zG1n4wPkSQ9dNpvoDMcqbflta85LdkO MQcWNJr0dHO5XzcjZG0zm0nybAoI9I5KSeH80lWB4uIQwghX1DiwoumVa+/oiSo0JBp5 5Vim7M5ruLAjtTeH9ayl5gu4AuG8Gm+lVKAiokj4NzIw+t372udVt2OT5Yo2hg86zCsW WRJUPdD/+FMp4HK+jfMKaXM/8mBYaS3U0394GkdY0LjM1DBTIn0/g0xqoNooJrjxipMY JrUA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-unsubscribe:list-subscribe:list-archive:list-help:list-post :list-id:mailing-list:precedence:mime-version:message-id:date :subject:cc:to:from:sender:dkim-signature; bh=zi4hAbNUPoS/lAF4LGVHNi2JXSWauuOe5Bk6hNSg2+I=; b=LabQcZSZnL0ml4/Alay/tEWYZX0aM9mRk7+9+k2Gr8Wvp3xUuRRD6rlv6EFJLQx9AC JFJehMzfKCFc04OWEfId9bB3TnClXzNAZ69n0P6MuovpsNiwAUYJiuoDYN0/gEMeKOZo HNkwngdwyj27i/4CefIIlsluQjyGVVLPdmq3oo09jtIVGF4tXtkcan0+fmZM4GneMOMn zOKQ9sSk0qgL8R+onXTAqZuwugSe5S0fIuGdi/B8ee9e5mHq3oEFvGVQIcl83IjnUgU+ xsFwdI3iVd9+24C4lv35vAOJ3xmC/qMiHr45S1AbnCjFca1Z87ifHb7OFIR/NpUHaKOR iagg== ARC-Authentication-Results: i=2; gmr-mx.google.com; dkim=pass header.i=@teufel.de header.s=hse1 header.b="VQXuk/WL"; spf=pass (google.com: domain of sava.jakovljev@teufel.de designates 94.100.136.155 as permitted sender) smtp.mailfrom=sava.jakovljev@teufel.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlegroups.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:mime-version :x-original-sender:x-original-authentication-results:precedence :mailing-list:list-id:list-post:list-help:list-archive :list-subscribe:list-unsubscribe; bh=zi4hAbNUPoS/lAF4LGVHNi2JXSWauuOe5Bk6hNSg2+I=; b=g5i80fnFtKMZicRGVfD6/SXj4AixV1xETDagrWInmw72fljLyARscqQN3cDgz7vucb JkBccWSkdEChprN6gfpUQ2mf0YE2wQfscFXxYNFLq055aqlqb7OziRLIgYjW4nERjze+ e+VGgJKM6INhxFzjf2/NuaO/xVazOtgh5xfHOINqI9hNrHkTIWEDVKlXvQqocXJW/W0e Kh3wrin17Dws6XdumqEY+XWbqL4EIcxO00jqwqFroCt8Pu8GsEFjpu1ZdtLbVak35du6 RcDjQXGUELpSgwHTiYCjtgEnMn/WSICDT7VyZ15pTI9P1fhz2mmT7AuqZzoHziks+pZQ G0rQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=sender:x-gm-message-state:from:to:cc:subject:date:message-id :mime-version:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:x-spam-checked-in-group:list-post :list-help:list-archive:list-subscribe:list-unsubscribe; bh=zi4hAbNUPoS/lAF4LGVHNi2JXSWauuOe5Bk6hNSg2+I=; b=TWw3ivkS3Dlx040stTA+hdhRmDh1gUWS3fhc8RmKZ22Q69VsJrfuOUnUQC0IYiOdvG jAe6EPy36ViKU8VD/2wVg4gdOSYWbRyh4ISyTuiZpw7tFi9TbrDREBAJHvhVikk6PlyK 2khwtDcfPbGkmQ53SRulB/zIlsCuk7ExdG6CXkWiRI9wElr3YfV2HGqOfdcqSL5+yHfZ ZuYCQOM8F/hc5ebjpw8uZdm+AUpi2Of9ltvyYFZvkl1jgPW+v8/6M6vPsRY6wz5BjQ2c qsNrPi//0loZNICyWOG2ze7x+H4peKERlW/deN88dIJhA8iXEgpWmRtZ8KsBQHL1Dypg OqJA== Sender: swupdate@googlegroups.com X-Gm-Message-State: AOAM532km+8AU9b/2+l6FLy6VZv9CKC+TA2fXdztDtwp2aJUu/OCoPtP P9uhtxWpmPwNcqQlAK8Q9sI= X-Google-Smtp-Source: ABdhPJwEPPdgfga/azwuWK/PpQhGFpd/P0QT3S9I49bmpl5qPjmWmRhvuRIfUyMZrA39ERaydo3p4g== X-Received: by 2002:a1c:b742:: with SMTP id h63mr10242018wmf.122.1607637158325; Thu, 10 Dec 2020 13:52:38 -0800 (PST) X-BeenThere: swupdate@googlegroups.com Received: by 2002:a1c:1fc7:: with SMTP id f190ls3678264wmf.1.gmail; Thu, 10 Dec 2020 13:52:37 -0800 (PST) X-Received: by 2002:a7b:c85a:: with SMTP id c26mr10040101wml.160.1607637157476; Thu, 10 Dec 2020 13:52:37 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1607637157; cv=none; d=google.com; s=arc-20160816; b=Mr5tbsNJeXuTv/gk7A/W+M0TD3J3Zs2wlHdQVkHpw9KHbIXkqYQKvq6z2SMKtyVhvE 6/hDjZZmzOq6ht0NdU+Cv3YlKS4K9OY7mFoYZ51+7TI1JUi2kMSRjmfCJVH5JdLrR5tp X3SL7L1VMkk1trLAAZ7vdxu3eND6RKMz0yhJaHHzWBkuzof/SkqC1hVjPpgqmCfYTAhS k8oHiT8qhNwHNysdAs6YUYRY9pnrTho7h4yBs6RG3QpuyOamwYNKLiXqfLbGpOedSTAM 3fE2FY51TE8vxczWE4Gb+q7Vmtg3IZmn51qW1kA1sCdyjYDJ/3jv/3JjvXATEsyelaCi 8r0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=dkim-signature:content-transfer-encoding:mime-version:message-id :date:subject:cc:to:from; bh=bs79klv7hiupMIHg2dawuPn46kEJSsXwhs0m9mlLeRE=; b=V7B9QO+8nxWy2HzYSHP+qP0NPlj6II6XXxA7vQ/ATDYkiuiNgV2Cb6kyP3iwxeeDeZ pE1DrlrLFR1Ge3vIdNwM3Jyu3hGclsKzVcoD5c1e1tGpGrPW75hm9VFuILxPPiVKI//0 lM+SQxuXu5gTTnqF6Df09GAl6mWJx3iRv3PRAARNGlgKHDcDaR7SSeLCCgvlUYUuf6Jb O3bG7yXeKFjNariWikCuJ+d4gX82BfVVfyl4j0/7oNSfbkcTsZgwRlATu0kGOd9hhmaF te+WwPLPk6mexYTvPYSvT8Py/mngk23powCkTk04rg1Ck6y3AThSozEJmGelt2KDTjZj Co6Q== ARC-Authentication-Results: i=1; gmr-mx.google.com; dkim=pass header.i=@teufel.de header.s=hse1 header.b="VQXuk/WL"; spf=pass (google.com: domain of sava.jakovljev@teufel.de designates 94.100.136.155 as permitted sender) smtp.mailfrom=sava.jakovljev@teufel.de Received: from mx-relay55-hz2.antispameurope.com (mx-relay55-hz2.antispameurope.com. [94.100.136.155]) by gmr-mx.google.com with ESMTPS id n8si266268wrr.0.2020.12.10.13.52.37 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 10 Dec 2020 13:52:37 -0800 (PST) Received-SPF: pass (google.com: domain of sava.jakovljev@teufel.de designates 94.100.136.155 as permitted sender) client-ip=94.100.136.155; Received: from unknown ([212.91.255.190]) by mx-relay55-hz2.antispameurope.com; Thu, 10 Dec 2020 22:52:37 +0100 From: Sava Jakovljev To: CC: Sava Jakovljev Subject: [swupdate] [PATCH] Make access to bootloader state exclusive also for GET and UNSET Date: Thu, 10 Dec 2020 22:52:18 +0100 Message-ID: <20201210215218.646959-1-sava.jakovljev@teufel.de> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 X-Originating-IP: [10.10.25.44] X-ClientProxiedBy: DNS-EX-02.teufel.local (10.10.0.81) To DNS-EX-02.teufel.local (10.10.0.81) X-C2ProcessedOrg: b93e13a0-e8da-4ba4-97b8-f14375b21c41 X-cloud-security-sender: sava.jakovljev@teufel.de X-cloud-security-recipient: swupdate@googlegroups.com X-cloud-security-Virusscan: CLEAN X-cloud-security-disclaimer: This E-Mail was scanned by E-Mailservice on mx-relay55-hz2.antispameurope.com with D52825E218D X-cloud-security-connect: unknown[212.91.255.190], TLS=1, IP=212.91.255.190 X-cloud-security: scantime:.2950 X-Original-Sender: sava.jakovljev@teufel.de X-Original-Authentication-Results: gmr-mx.google.com; dkim=pass header.i=@teufel.de header.s=hse1 header.b="VQXuk/WL"; spf=pass (google.com: domain of sava.jakovljev@teufel.de designates 94.100.136.155 as permitted sender) smtp.mailfrom=sava.jakovljev@teufel.de 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: , * Introduce IPC calls GET_UPDATE_STATE and UNSET_UPDATE_STATE. * Use plain Linux-style return values from function and keep things simple. Signed-off-by: Sava Jakovljev --- When RAM bootloader environment is used, get_state() from subprocesses like Suricatta is broken, since they will have their own copy of the dictionary structure (defined in bootloader/none.c). Semantically the main process is owning the state, thus introducing these new IPC calls fit to that design. Tests are updated accordingly. Also, return values of the functions are simplified. core/network_thread.c | 13 +++++- core/state.c | 86 ++++++++++++++++++++++++++------------ include/network_ipc.h | 6 ++- include/state.h | 7 ++-- test/test_server_hawkbit.c | 33 ++++++--------- 5 files changed, 90 insertions(+), 55 deletions(-) diff --git a/core/network_thread.c b/core/network_thread.c index 0e6080d..ae4eb24 100644 --- a/core/network_thread.c +++ b/core/network_thread.c @@ -87,9 +87,9 @@ static bool is_selection_allowed(const char *software_set, char *running_mode, if (allowed) { INFO("Accepted selection %s,%s", software_set, running_mode); - }else + }else ERROR("Selection %s,%s is not allowed, rejected !", - software_set, running_mode); + software_set, running_mode); return allowed; } @@ -445,6 +445,15 @@ void *network_thread (void *data) ? ACK : NACK; break; + case GET_UPDATE_STATE: + msg.data.msg[0] = get_state(); + msg.type = ACK; + break; + case UNSET_UPDATE_STATE: + msg.type = unset_state(msg.data.msg[0]) == 0 ? + ACK : NACK; + + break; default: msg.type = NACK; } diff --git a/core/state.c b/core/state.c index 8ddd760..74c1d55 100644 --- a/core/state.c +++ b/core/state.c @@ -30,19 +30,19 @@ } \ } while(0) -static server_op_res_t do_save_state(char *key, char* value) +static int do_save_state(char *key, char* value) { - char c; CHECK_STATE_VAR(key); if (!value) return -EINVAL; - c = *value; + char c = *value; if (c < STATE_OK || c > STATE_LAST) return -EINVAL; - return bootloader_env_set(key, value) == 0 ? SERVER_OK : SERVER_EERR; + + return bootloader_env_set(key, value); } -server_op_res_t save_state(char *key, update_state_t value) +int save_state(char *key, update_state_t value) { char value_str[2] = {value, '\0'}; ipc_message msg; @@ -52,64 +52,96 @@ server_op_res_t save_state(char *key, update_state_t value) msg.type = SET_UPDATE_STATE; msg.data.msg[0] = (char)value; return (ipc_send_cmd(&msg)); - } else { /* Main process */ + } else { + /* Main process */ return do_save_state(key, value_str); } } -server_op_res_t save_state_string(char *key, update_state_t value) +int save_state_string(char *key, update_state_t value) { CHECK_STATE_VAR(key); if (!value) return -EINVAL; if (value < STATE_OK || value > STATE_LAST) return -EINVAL; - return bootloader_env_set(key, get_state_string(value)) == 0 ? - SERVER_OK : SERVER_EERR; + return bootloader_env_set(key, get_state_string(value)); } -server_op_res_t read_state(char *key, update_state_t *value) +static update_state_t read_state(char *key) { - char *envval; CHECK_STATE_VAR(key); - envval = bootloader_env_get(key); + char *envval = bootloader_env_get(key); if (envval == NULL) { INFO("Key '%s' not found in Bootloader's environment.", key); - *value = STATE_NOT_AVAILABLE; - return SERVER_OK; + return STATE_NOT_AVAILABLE; } /* TODO It's a bit whacky just to cast this but as we're the only */ /* ones touching the variable, it's maybe OK for a PoC now. */ - *value = (update_state_t)*envval; + update_state_t val = (update_state_t)*envval; /* bootloader get env allocates space for the value */ free(envval); - return SERVER_OK; + return val; } -server_op_res_t unset_state(char *key) -{ - int ret; +static int do_unset_state(char *key) +{ CHECK_STATE_VAR(key); - ret = bootloader_env_unset(key); - return ret == 0 ? SERVER_OK : SERVER_EERR; + return bootloader_env_unset(key); } -update_state_t get_state(void) { - update_state_t state; +int unset_state(char *key) +{ + if (pid == getpid()) + { + ipc_message msg; + memset(&msg, 0, sizeof(msg)); + + msg.type = UNSET_UPDATE_STATE; + return ipc_send_cmd(&msg) == 0 && msg.type == ACK; + } else { + // Main process + return do_unset_state(key); + } +} + +static update_state_t do_get_state(void) { + update_state_t state = read_state((char *)STATE_KEY); - if (read_state((char *)STATE_KEY, &state) != SERVER_OK) { + if (state == STATE_NOT_AVAILABLE) { ERROR("Cannot read stored update state."); - return STATE_ERROR; + return STATE_NOT_AVAILABLE; } - TRACE("Read state=%c from persistent storage.", state); if (is_valid_state(state)) { + TRACE("Read state=%c from persistent storage.", state); return state; } + ERROR("Unknown update state=%c", state); - return STATE_ERROR; + return STATE_NOT_AVAILABLE; +} + +update_state_t get_state(void) { + if (pid == getpid()) + { + ipc_message msg; + memset(&msg, 0, sizeof(msg)); + + msg.type = GET_UPDATE_STATE; + + if (ipc_send_cmd(&msg) || msg.type == NACK) { + ERROR("Failed to get current bootloader update state."); + return STATE_NOT_AVAILABLE; + } + + return (update_state_t)msg.data.msg[0]; + } else { + // Main process + return do_get_state(); + } } diff --git a/include/network_ipc.h b/include/network_ipc.h index 22ecb9a..34bc43d 100644 --- a/include/network_ipc.h +++ b/include/network_ipc.h @@ -35,6 +35,8 @@ typedef enum { SWUPDATE_SUBPROCESS, SET_AES_KEY, SET_UPDATE_STATE, /* set bootloader ustate */ + GET_UPDATE_STATE, + UNSET_UPDATE_STATE, REQ_INSTALL_EXT } msgtype; @@ -71,7 +73,7 @@ struct swupdate_request { typedef union { char msg[128]; - struct { + struct { int current; int last_result; int error; @@ -100,7 +102,7 @@ typedef union { char ivt_ascii[33]; /* Key size in ASCII (16 bytes bin) + termination */ } aeskeymsg; } msgdata; - + typedef struct { int magic; /* magic number */ int type; diff --git a/include/state.h b/include/state.h index f006b6c..cb9a947 100644 --- a/include/state.h +++ b/include/state.h @@ -63,8 +63,7 @@ static inline char* get_state_string(update_state_t state) { return (char*)""; } -server_op_res_t save_state(char *key, update_state_t value); -server_op_res_t save_state_string(char *key, update_state_t value); -server_op_res_t read_state(char *key, update_state_t *value); -server_op_res_t unset_state(char *key); +int save_state(char *key, update_state_t value); +int save_state_string(char *key, update_state_t value); +int unset_state(char *key); update_state_t get_state(void); diff --git a/test/test_server_hawkbit.c b/test/test_server_hawkbit.c index 9c7c128..03c5475 100644 --- a/test/test_server_hawkbit.c +++ b/test/test_server_hawkbit.c @@ -115,22 +115,20 @@ channel_op_res_t __wrap_channel_get(channel_t *this, void *data) return mock_type(channel_op_res_t); } -extern server_op_res_t __real_save_state(char *key, update_state_t value); -server_op_res_t __wrap_save_state(char *key, update_state_t *value); -server_op_res_t __wrap_save_state(char *key, update_state_t *value) +extern int __real_save_state(char *key, update_state_t value); +int __wrap_save_state(char *key, update_state_t *value); +int __wrap_save_state(char *key, update_state_t *value) { (void)key; (void)value; - return mock_type(server_op_res_t); + return mock_type(int); } -extern server_op_res_t __real_read_state(char *key, update_state_t *value); -server_op_res_t __wrap_read_state(char *key, update_state_t *value); -server_op_res_t __wrap_read_state(char *key, update_state_t *value) +extern update_state_t __real_get_state(void); +update_state_t __wrap_get_state(void); +update_state_t __wrap_get_state(void) { - (void)key; - *value = mock_type(update_state_t); - return mock_type(server_op_res_t); + return mock_type(update_state_t); } extern server_op_res_t server_has_pending_action(int *action_id); @@ -218,14 +216,11 @@ static void test_server_has_pending_action(void **state) will_return(__wrap_channel_get, json_tokener_parse(json_reply_update_data)); will_return(__wrap_channel_get, CHANNEL_OK); -#if 0 - will_return(__wrap_read_state, STATE_NOT_AVAILABLE); - will_return(__wrap_read_state, SERVER_OK); -#endif + + will_return(__wrap_get_state, STATE_NOT_AVAILABLE); assert_int_equal(SERVER_UPDATE_AVAILABLE, server_has_pending_action(&action_id)); -#if 0 /* Test Case: Update Action available && STATE_INSTALLED. */ will_return(__wrap_channel_get, json_tokener_parse(json_reply_update_available)); @@ -233,11 +228,9 @@ static void test_server_has_pending_action(void **state) will_return(__wrap_channel_get, json_tokener_parse(json_reply_update_data)); will_return(__wrap_channel_get, CHANNEL_OK); - will_return(__wrap_read_state, STATE_INSTALLED); - will_return(__wrap_read_state, SERVER_OK); + will_return(__wrap_get_state, STATE_INSTALLED); assert_int_equal(SERVER_NO_UPDATE_AVAILABLE, server_has_pending_action(&action_id)); -#endif /* Test Case: Cancel Action available. */ will_return(__wrap_channel_get, @@ -247,7 +240,7 @@ static void test_server_has_pending_action(void **state) json_tokener_parse(json_reply_cancel_data)); will_return(__wrap_channel_get, CHANNEL_OK); will_return(__wrap_channel_put, CHANNEL_OK); - will_return(__wrap_save_state, SERVER_OK); + will_return(__wrap_save_state, 0); assert_int_equal(SERVER_OK, server_has_pending_action(&action_id)); } @@ -542,7 +535,7 @@ static void test_server_install_update(void **state) will_return(__wrap_channel_get_file, CHANNEL_OK); will_return(__wrap_ipc_wait_for_complete, SUCCESS); will_return(__wrap_channel_put, CHANNEL_OK); - //will_return(__wrap_save_state, SERVER_OK); + //will_return(__wrap_save_state, 0); will_return(__wrap_channel_put, CHANNEL_OK); assert_int_equal(SERVER_OK, server_install_update()); }