| Message ID | 20210126130556.21616-1-christian.storm@siemens.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | state: Optionally disable setting ustate marker | expand |
Hi Christian, On 26.01.21 14:05, Christian Storm wrote: > Analogously to the bootloader_transaction_marker, introduce a > bootloader_state_marker that allows to disable setting ustate. > This is useful if the installed update does not require persistent > state handling at all like, e.g., applications, or when using > a single-copy setup. But if it is not necessary, why don't you simply set CONFIG_BOOTLOADER_NONE ? This disables the persistency. Best regards, Stefano > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > core/stream_interface.c | 8 ++++++-- > core/swupdate.c | 8 +++++++- > doc/source/sw-description.rst | 13 ++++++++++--- > doc/source/swupdate.rst | 3 +++ > include/swupdate.h | 2 ++ > parser/parser.c | 11 +++++++++++ > 6 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/core/stream_interface.c b/core/stream_interface.c > index 0205866..3907ade 100644 > --- a/core/stream_interface.c > +++ b/core/stream_interface.c > @@ -603,7 +603,9 @@ void *network_initializer(void *data) > } > notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); > inst.last_install = FAILURE; > - if (!software->globals.dry_run && save_state((char *)STATE_KEY, STATE_FAILED) != SERVER_OK) { > + if (!software->globals.dry_run > + && software->bootloader_state_marker > + && save_state((char *)STATE_KEY, STATE_FAILED) != SERVER_OK) { > WARN("Cannot persistently store FAILED update state."); > } > } else { > @@ -614,7 +616,9 @@ void *network_initializer(void *data) > if (!software->globals.dry_run && software->bootloader_transaction_marker) { > bootloader_env_unset(BOOTVAR_TRANSACTION); > } > - if (!software->globals.dry_run && save_state((char *)STATE_KEY, STATE_INSTALLED) != SERVER_OK) { > + if (!software->globals.dry_run > + && software->bootloader_state_marker > + && save_state((char *)STATE_KEY, STATE_INSTALLED) != SERVER_OK) { > ERROR("Cannot persistently store INSTALLED update state."); > notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); > inst.last_install = FAILURE; > diff --git a/core/swupdate.c b/core/swupdate.c > index 4f008e0..62fc085 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -83,6 +83,7 @@ static struct option long_options[] = { > {"no-downgrading", required_argument, NULL, 'N'}, > {"no-reinstalling", required_argument, NULL, 'R'}, > {"no-transaction-marker", no_argument, NULL, 'M'}, > + {"no-state-marker", no_argument, NULL, 'm'}, > #ifdef CONFIG_SIGNED_IMAGES > {"key", required_argument, NULL, 'k'}, > {"ca-path", required_argument, NULL, 'k'}, > @@ -153,6 +154,7 @@ static void usage(char *programname) > " -N, --no-downgrading <version> : not install a release older as <version>\n" > " -R, --no-reinstalling <version>: not install a release same as <version>\n" > " -M, --no-transaction-marker : disable setting bootloader transaction marker\n" > + " -m, --no-state-marker : disable setting update state in bootloader\n" > " -o, --output <filename> : saves the incoming stream\n" > " -v, --verbose : be verbose, set maximum loglevel\n" > " --version : print SWUpdate version and exit\n" > @@ -470,7 +472,7 @@ int main(int argc, char **argv) > #endif > memset(main_options, 0, sizeof(main_options)); > memset(image_url, 0, sizeof(image_url)); > - strcpy(main_options, "vhni:e:q:l:Lcf:p:P:o:N:R:M"); > + strcpy(main_options, "vhni:e:q:l:Lcf:p:P:o:N:R:Mm"); > #ifdef CONFIG_MTD > strcat(main_options, "b:"); > #endif > @@ -640,6 +642,10 @@ int main(int argc, char **argv) > swcfg.globals.no_transaction_marker = 1; > TRACE("transaction_marker globally disabled"); > break; > + case 'm': > + swcfg.globals.no_state_marker = 1; > + TRACE("state_marker globally disabled"); > + break; > case 'e': > software_select = optarg; > opt_e = 1; > diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst > index 63be19f..fd8bd11 100644 > --- a/doc/source/sw-description.rst > +++ b/doc/source/sw-description.rst > @@ -832,8 +832,8 @@ SWUpdate scans for all scripts and calls them after installing the images. > If the data attribute is defined, its value is passed as the last argument(s) > to the script. > > -Update Transaction Marker > -------------------------- > +Update Transaction and Status Marker > +------------------------------------ > > By default, SWUpdate sets the bootloader environment variable "recovery_status" > to "in_progress" prior to an update operation and either unsets it or sets it to > @@ -861,7 +861,14 @@ transaction marker: > > It is also possible to disable setting of the transaction marker > entirely (and independently of the setting in `sw-description`) by > -starting swupdate with the `-M` option. > +starting SWUpdate with the `-M` option. > + > + > +The same applies to setting the update state in the bootloader via its > +environment variable "ustate" (default) to `STATE_INSTALLED=1` or > +`STATE_FAILED=3` after an installation. This behavior can be turned off > +globally via the `-m` option to SWUpdate or per `sw-description` via the > +boolean switch "bootloader_state_marker". > > bootloader > ---------- > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > index a8144d1..22d135a 100644 > --- a/doc/source/swupdate.rst > +++ b/doc/source/swupdate.rst > @@ -518,6 +518,9 @@ Command line parameters > | -M | - | Disable setting the bootloader transaction | > | | | marker. | > +-------------+----------+--------------------------------------------+ > +| -m | - | Disable setting the update state in the | > +| | | bootloader. | > ++-------------+----------+--------------------------------------------+ > | -w <parms> | string | Available if CONFIG_WEBSERVER is set. | > | | | Start internal webserver and pass to it | > | | | a command line string. | > diff --git a/include/swupdate.h b/include/swupdate.h > index 137d8f2..e607ed6 100644 > --- a/include/swupdate.h > +++ b/include/swupdate.h > @@ -131,6 +131,7 @@ struct swupdate_global_cfg { > int no_downgrading; > int no_reinstalling; > int no_transaction_marker; > + int no_state_marker; > char publickeyfname[SWUPDATE_GENERAL_STRING_SIZE]; > char aeskeyfname[SWUPDATE_GENERAL_STRING_SIZE]; > char postupdatecmd[SWUPDATE_GENERAL_STRING_SIZE]; > @@ -149,6 +150,7 @@ struct swupdate_cfg { > char description[SWUPDATE_UPDATE_DESCRIPTION_STRING_SIZE]; > char version[SWUPDATE_GENERAL_STRING_SIZE]; > bool bootloader_transaction_marker; > + bool bootloader_state_marker; > char software_set[SWUPDATE_GENERAL_STRING_SIZE]; > char running_mode[SWUPDATE_GENERAL_STRING_SIZE]; > char output[SWUPDATE_GENERAL_STRING_SIZE]; > diff --git a/parser/parser.c b/parser/parser.c > index c2fe175..d3efde5 100644 > --- a/parser/parser.c > +++ b/parser/parser.c > @@ -148,6 +148,17 @@ static bool get_common_fields(parsertype p, void *cfg, struct swupdate_cfg *swcf > TRACE("Description %s", swcfg->description); > } > > + if(swcfg->globals.no_state_marker) { > + swcfg->bootloader_state_marker = false; > + } else { > + swcfg->bootloader_state_marker = true; > + if((setting = find_node(p, cfg, "bootloader_state_marker", swcfg)) != NULL) { > + get_field(p, setting, NULL, &swcfg->bootloader_state_marker); > + TRACE("Setting bootloader state marker: %s", > + swcfg->bootloader_state_marker == true ? "true" : "false"); > + } > + } > + > if(swcfg->globals.no_transaction_marker) { > swcfg->bootloader_transaction_marker = false; > } else { >
Hi Stefano, > > Analogously to the bootloader_transaction_marker, introduce a > > bootloader_state_marker that allows to disable setting ustate. > > This is useful if the installed update does not require persistent > > state handling at all like, e.g., applications, or when using > > a single-copy setup. > > But if it is not necessary, why don't you simply set > CONFIG_BOOTLOADER_NONE ? This disables the persistency. Yes, true. However, if you want to install (1) firmware ― requires persistent state handling, e.g., ustate=1 for bootloader interaction / instrumentation ― as well as (2) applications that do *not* require this ― but instead, e.g., a SIGHUP to some daemon or whatever ― via *one* SWUpdate instance, then disabling both, bootloader_transaction_marker and bootloader_state_marker (in sw-description) will bring you there. The feature is to not touch the bootloader persistent store at all for artifacts that don't require bootloader interaction / instrumentation, for safety reasons: Without this, you would have to reset ustate to 0 after having installed such an artifact, writing two times to the bootloader environment although none is necessary. Kind regards, Christian
Hi Christian, On 26.01.21 15:39, Christian Storm wrote: > Hi Stefano, > >>> Analogously to the bootloader_transaction_marker, introduce a >>> bootloader_state_marker that allows to disable setting ustate. >>> This is useful if the installed update does not require persistent >>> state handling at all like, e.g., applications, or when using >>> a single-copy setup. >> >> But if it is not necessary, why don't you simply set >> CONFIG_BOOTLOADER_NONE ? This disables the persistency. > > Yes, true. However, if you want to install (1) firmware ― requires > persistent state handling, e.g., ustate=1 for bootloader interaction / > instrumentation ― as well as (2) applications that do *not* require this > ― but instead, e.g., a SIGHUP to some daemon or whatever ― via *one* > SWUpdate instance, then disabling both, bootloader_transaction_marker > and bootloader_state_marker (in sw-description) will bring you there. Then you are saying you need a per-install setup while the daemon (SWUpdate) is still running, while you set it just at the startup. This option should the be triggered by an attribute in sw-description (like "output") or by an IPC instead of command line parameters, that constraints to stop and restart the daemon. So I am not sure if putting "no-state-marker" is a good idea, I agree to let it in sw-description (also covered by your patch). Do we need it as command line parameter ? Or better : I am just concerned if we can have security drawbacks for this. What do you think ? > > The feature is to not touch the bootloader persistent store at all for > artifacts that don't require bootloader interaction / instrumentation, > for safety reasons: Without this, you would have to reset ustate to > 0 after having installed such an artifact, writing two times to the > bootloader environment although none is necessary. Right, or introduce some more logic (this is what I did in this use case), that is with additional variables to identify the install type. I am not against this change, I have just security concerns (maybe too paranoid ?). Best regards, Stefano
Hi Stefano, > >>> Analogously to the bootloader_transaction_marker, introduce a > >>> bootloader_state_marker that allows to disable setting ustate. > >>> This is useful if the installed update does not require persistent > >>> state handling at all like, e.g., applications, or when using > >>> a single-copy setup. > >> > >> But if it is not necessary, why don't you simply set > >> CONFIG_BOOTLOADER_NONE ? This disables the persistency. > > > > Yes, true. However, if you want to install (1) firmware ― requires > > persistent state handling, e.g., ustate=1 for bootloader interaction / > > instrumentation ― as well as (2) applications that do *not* require this > > ― but instead, e.g., a SIGHUP to some daemon or whatever ― via *one* > > SWUpdate instance, then disabling both, bootloader_transaction_marker > > and bootloader_state_marker (in sw-description) will bring you there. > > Then you are saying you need a per-install setup while the daemon > (SWUpdate) is still running, Yes, I want to install firmware and applications with the same SWUpdate instance without restart. Firmware requires persistent state handling while applications do not, and, moreover, the bootloader should not be touched at all in this case for safety. > while you set it just at the startup. Well, yes and no. For the above use case, I'd use in the sw-description bootloader_transaction_marker = false bootloader_state_marker = false for applications to not touch the bootloader at all while I'd use bootloader_transaction_marker = false [bootloader_state_marker = true → not set there explicitly, default] for firmware (since I just need ustate in the double-copy approach). The activation (reboot or SIGHUP or whatever) is then handled by a progress client. At startup, I do not set anything (no -M and no -m), so effectively: bootloader_transaction_marker = true bootloader_state_marker = true > This option should the be triggered by an attribute in sw-description (like > "output") or by an IPC instead of command line parameters, that > constraints to stop and restart the daemon. > > So I am not sure if putting "no-state-marker" is a good idea, I agree to > let it in sw-description (also covered by your patch). > > Do we need it as command line parameter ? Or better : I am just > concerned if we can have security drawbacks for this. What do you think ? Hm, I do not necessarily need it for my use cases. But, currently, there's -M with which you can shoot yourself in the foot in single-copy scenarios. This is the same but for double-copy scenarios. So, the integrator has to have the knowledge to not hurt his feet in any case ― which you can't prevent anyway if you give him options, even in the sw-description which is done by just another integrator having to have the knowledge... Second, consider the distro use case, e.g., Debian: You have one binary which you configure at run-time with switches to your needs instead of at compile-time. Without the (new) -m switch, the bootloader will be superfluously touched (twice) on application installs, which is at least not beneficial to safety. Third, I think security is not affected at all by this. The worst thing that could happen is that you erroneously use -m (or/and -M) and your update workflow doesn't play out. Here, the first argument applies. Or do I miss some security implication? Fourth, for single-copy scenarios, you may not want to superfluously set ustate as you don't need it for this purpose. > > The feature is to not touch the bootloader persistent store at all for > > artifacts that don't require bootloader interaction / instrumentation, > > for safety reasons: Without this, you would have to reset ustate to > > 0 after having installed such an artifact, writing two times to the > > bootloader environment although none is necessary. > > Right, or introduce some more logic (this is what I did in this use > case), that is with additional variables to identify the install type. Even so, ustate is still unconditionally set by SWUpdate core (core/ stream_interface.c, lines 610ff) and I have to reset it (ustate = STATE_OK) after having installed an application so that on the next boot the bootloader doesn't erroneously think I did a firmware update and switches (back) to the other installation in a double-copy scenario. If you consider downgrading a security implication, then you will want to have this option (used correctly, of course), either as command line switch or/and in sw-description. Having different update types doesn't solve this problem, just the problem of different activation methods, i.e., reboot for firmware, SIGHUP for applications etc. pp. Say you have type="application", then your progress activation client does the SIGHUP and does have to set ustate = STATE_OK to not fool the bootloader (or you can do this in postinstall etc. but the point is still you unnecessarily touch the bootloader). Or did I miss something here? > I am not against this change, I have just security concerns (maybe too > paranoid ?). No worries, I'm happy to discuss this, it only improves quality! Kind regards, Christian
Hi Christian, On 26.01.21 16:57, Christian Storm wrote: > Hi Stefano, > >>>>> Analogously to the bootloader_transaction_marker, introduce a >>>>> bootloader_state_marker that allows to disable setting ustate. >>>>> This is useful if the installed update does not require persistent >>>>> state handling at all like, e.g., applications, or when using >>>>> a single-copy setup. >>>> >>>> But if it is not necessary, why don't you simply set >>>> CONFIG_BOOTLOADER_NONE ? This disables the persistency. >>> >>> Yes, true. However, if you want to install (1) firmware ― requires >>> persistent state handling, e.g., ustate=1 for bootloader interaction / >>> instrumentation ― as well as (2) applications that do *not* require this >>> ― but instead, e.g., a SIGHUP to some daemon or whatever ― via *one* >>> SWUpdate instance, then disabling both, bootloader_transaction_marker >>> and bootloader_state_marker (in sw-description) will bring you there. >> >> Then you are saying you need a per-install setup while the daemon >> (SWUpdate) is still running, > > Yes, I want to install firmware and applications with the same SWUpdate > instance without restart. Firmware requires persistent state handling > while applications do not, and, moreover, the bootloader should not > be touched at all in this case for safety. > > >> while you set it just at the startup. > > Well, yes and no. For the above use case, I'd use in the sw-description > bootloader_transaction_marker = false > bootloader_state_marker = false > for applications to not touch the bootloader at all while I'd use > bootloader_transaction_marker = false > [bootloader_state_marker = true → not set there explicitly, default] > for firmware (since I just need ustate in the double-copy approach). > The activation (reboot or SIGHUP or whatever) is then handled by > a progress client. > At startup, I do not set anything (no -M and no -m), so effectively: > bootloader_transaction_marker = true > bootloader_state_marker = true > > >> This option should the be triggered by an attribute in sw-description (like >> "output") or by an IPC instead of command line parameters, that >> constraints to stop and restart the daemon. >> >> So I am not sure if putting "no-state-marker" is a good idea, I agree to >> let it in sw-description (also covered by your patch). >> >> Do we need it as command line parameter ? Or better : I am just >> concerned if we can have security drawbacks for this. What do you think ? > > Hm, I do not necessarily need it for my use cases. But, currently, > there's -M with which you can shoot yourself in the foot in single-copy > scenarios. Well, command line parameters must be evaluated by the integrator and should be set correct, preferably in the systemd run unit or startup script (in meta-swupdate via configuration file that are included by the startup script). If -M seems dangerous, I could start with selection pointing to the running copy, that destroys the system. > This is the same but for double-copy scenarios. So, the > integrator has to have the knowledge to not hurt his feet in any case “You cannot make things foolproof because fools are so ingenious” We cannot help if the integrator does not know what he is doing. > ― which you can't prevent anyway if you give him options, even in the > sw-description which is done by just another integrator having to have > the knowledge... Right - I exclude this case. If he cannot, he should find someone who can. I am just thinking cases where SWUpdate is configured correctly, but it is possible to set something at runtime that breaks. Wrong values in sw-description are not covered, too: sw-description is signed (strongly suggested), and the integrator should know what he's doing. > Second, consider the distro use case, e.g., Debian: You have one binary > which you configure at run-time with switches to your needs instead of > at compile-time. Ok > Without the (new) -m switch, the bootloader will be > superfluously touched (twice) on application installs, which is at least > not beneficial to safety. > Third, I think security is not affected at all by this. The worst thing > that could happen is that you erroneously use -m (or/and -M) and your > update workflow doesn't play out. Here, the first argument applies. Agree, I am convinced, sorry for noise. Your patch doesn't do anything different as we have. > Or do I miss some security implication? > Fourth, for single-copy scenarios, you may not want to superfluously set > ustate as you don't need it for this purpose. Well, it is not a big noise, but anyway there is nothing to prevent your patch. > > >>> The feature is to not touch the bootloader persistent store at all for >>> artifacts that don't require bootloader interaction / instrumentation, >>> for safety reasons: Without this, you would have to reset ustate to >>> 0 after having installed such an artifact, writing two times to the >>> bootloader environment although none is necessary. >> >> Right, or introduce some more logic (this is what I did in this use >> case), that is with additional variables to identify the install type. > > Even so, ustate is still unconditionally set by SWUpdate core (core/ > stream_interface.c, lines 610ff) and I have to reset it (ustate = STATE_OK) > after having installed an application so that on the next boot the > bootloader doesn't erroneously think I did a firmware update and > switches (back) to the other installation in a double-copy scenario. Yes, I get the point. I have managed this in the past with additional variable (set by the application installer), to convince the bootloader that it was an application update instead of OS update. Fine with me. > > If you consider downgrading a security implication, then you will want > to have this option (used correctly, of course), either as command line > switch or/and in sw-description. We have already a no-downgrading feature that checks versions, and setting U-Boot variables must be in some way protected (or the import of the variables, this is what was done in U-Boot). This is OT for this patch. > > Having different update types doesn't solve this problem, just the > problem of different activation methods, i.e., reboot for firmware, > SIGHUP for applications etc. pp. Say you have type="application", then > your progress activation client does the SIGHUP and does have to set > ustate = STATE_OK to not fool the bootloader (or you can do this in > postinstall etc. but the point is still you unnecessarily touch the > bootloader). In some ways, yes. > > Or did I miss something here? > > >> I am not against this change, I have just security concerns (maybe too >> paranoid ?). > > No worries, I'm happy to discuss this, it only improves quality! > Good, I got the point. I recheck your patch, but it looks fine to me. Regards, Stefano
Hi Stefano, > >>> [ ...] > >>> The feature is to not touch the bootloader persistent store at all for > >>> artifacts that don't require bootloader interaction / instrumentation, > >>> for safety reasons: Without this, you would have to reset ustate to > >>> 0 after having installed such an artifact, writing two times to the > >>> bootloader environment although none is necessary. > >> > >> Right, or introduce some more logic (this is what I did in this use > >> case), that is with additional variables to identify the install type. > > > > Even so, ustate is still unconditionally set by SWUpdate core (core/ > > stream_interface.c, lines 610ff) and I have to reset it (ustate = STATE_OK) > > after having installed an application so that on the next boot the > > bootloader doesn't erroneously think I did a firmware update and > > switches (back) to the other installation in a double-copy scenario. > > Yes, I get the point. I have managed this in the past with additional > variable (set by the application installer), to convince the bootloader > that it was an application update instead of OS update. Fine with me. Yes, if you have something powerful like UBoot with its scripting capabilities. However, not all bootloaders are that advanced though... > > If you consider downgrading a security implication, then you will want > > to have this option (used correctly, of course), either as command line > > switch or/and in sw-description. > > We have already a no-downgrading feature that checks versions, and > setting U-Boot variables must be in some way protected (or the import of > the variables, this is what was done in U-Boot). This is OT for this patch. Well, again thanks to the power of UBoot :) Kind regards, Christian
diff --git a/core/stream_interface.c b/core/stream_interface.c index 0205866..3907ade 100644 --- a/core/stream_interface.c +++ b/core/stream_interface.c @@ -603,7 +603,9 @@ void *network_initializer(void *data) } notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); inst.last_install = FAILURE; - if (!software->globals.dry_run && save_state((char *)STATE_KEY, STATE_FAILED) != SERVER_OK) { + if (!software->globals.dry_run + && software->bootloader_state_marker + && save_state((char *)STATE_KEY, STATE_FAILED) != SERVER_OK) { WARN("Cannot persistently store FAILED update state."); } } else { @@ -614,7 +616,9 @@ void *network_initializer(void *data) if (!software->globals.dry_run && software->bootloader_transaction_marker) { bootloader_env_unset(BOOTVAR_TRANSACTION); } - if (!software->globals.dry_run && save_state((char *)STATE_KEY, STATE_INSTALLED) != SERVER_OK) { + if (!software->globals.dry_run + && software->bootloader_state_marker + && save_state((char *)STATE_KEY, STATE_INSTALLED) != SERVER_OK) { ERROR("Cannot persistently store INSTALLED update state."); notify(FAILURE, RECOVERY_ERROR, ERRORLEVEL, "Installation failed !"); inst.last_install = FAILURE; diff --git a/core/swupdate.c b/core/swupdate.c index 4f008e0..62fc085 100644 --- a/core/swupdate.c +++ b/core/swupdate.c @@ -83,6 +83,7 @@ static struct option long_options[] = { {"no-downgrading", required_argument, NULL, 'N'}, {"no-reinstalling", required_argument, NULL, 'R'}, {"no-transaction-marker", no_argument, NULL, 'M'}, + {"no-state-marker", no_argument, NULL, 'm'}, #ifdef CONFIG_SIGNED_IMAGES {"key", required_argument, NULL, 'k'}, {"ca-path", required_argument, NULL, 'k'}, @@ -153,6 +154,7 @@ static void usage(char *programname) " -N, --no-downgrading <version> : not install a release older as <version>\n" " -R, --no-reinstalling <version>: not install a release same as <version>\n" " -M, --no-transaction-marker : disable setting bootloader transaction marker\n" + " -m, --no-state-marker : disable setting update state in bootloader\n" " -o, --output <filename> : saves the incoming stream\n" " -v, --verbose : be verbose, set maximum loglevel\n" " --version : print SWUpdate version and exit\n" @@ -470,7 +472,7 @@ int main(int argc, char **argv) #endif memset(main_options, 0, sizeof(main_options)); memset(image_url, 0, sizeof(image_url)); - strcpy(main_options, "vhni:e:q:l:Lcf:p:P:o:N:R:M"); + strcpy(main_options, "vhni:e:q:l:Lcf:p:P:o:N:R:Mm"); #ifdef CONFIG_MTD strcat(main_options, "b:"); #endif @@ -640,6 +642,10 @@ int main(int argc, char **argv) swcfg.globals.no_transaction_marker = 1; TRACE("transaction_marker globally disabled"); break; + case 'm': + swcfg.globals.no_state_marker = 1; + TRACE("state_marker globally disabled"); + break; case 'e': software_select = optarg; opt_e = 1; diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst index 63be19f..fd8bd11 100644 --- a/doc/source/sw-description.rst +++ b/doc/source/sw-description.rst @@ -832,8 +832,8 @@ SWUpdate scans for all scripts and calls them after installing the images. If the data attribute is defined, its value is passed as the last argument(s) to the script. -Update Transaction Marker -------------------------- +Update Transaction and Status Marker +------------------------------------ By default, SWUpdate sets the bootloader environment variable "recovery_status" to "in_progress" prior to an update operation and either unsets it or sets it to @@ -861,7 +861,14 @@ transaction marker: It is also possible to disable setting of the transaction marker entirely (and independently of the setting in `sw-description`) by -starting swupdate with the `-M` option. +starting SWUpdate with the `-M` option. + + +The same applies to setting the update state in the bootloader via its +environment variable "ustate" (default) to `STATE_INSTALLED=1` or +`STATE_FAILED=3` after an installation. This behavior can be turned off +globally via the `-m` option to SWUpdate or per `sw-description` via the +boolean switch "bootloader_state_marker". bootloader ---------- diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst index a8144d1..22d135a 100644 --- a/doc/source/swupdate.rst +++ b/doc/source/swupdate.rst @@ -518,6 +518,9 @@ Command line parameters | -M | - | Disable setting the bootloader transaction | | | | marker. | +-------------+----------+--------------------------------------------+ +| -m | - | Disable setting the update state in the | +| | | bootloader. | ++-------------+----------+--------------------------------------------+ | -w <parms> | string | Available if CONFIG_WEBSERVER is set. | | | | Start internal webserver and pass to it | | | | a command line string. | diff --git a/include/swupdate.h b/include/swupdate.h index 137d8f2..e607ed6 100644 --- a/include/swupdate.h +++ b/include/swupdate.h @@ -131,6 +131,7 @@ struct swupdate_global_cfg { int no_downgrading; int no_reinstalling; int no_transaction_marker; + int no_state_marker; char publickeyfname[SWUPDATE_GENERAL_STRING_SIZE]; char aeskeyfname[SWUPDATE_GENERAL_STRING_SIZE]; char postupdatecmd[SWUPDATE_GENERAL_STRING_SIZE]; @@ -149,6 +150,7 @@ struct swupdate_cfg { char description[SWUPDATE_UPDATE_DESCRIPTION_STRING_SIZE]; char version[SWUPDATE_GENERAL_STRING_SIZE]; bool bootloader_transaction_marker; + bool bootloader_state_marker; char software_set[SWUPDATE_GENERAL_STRING_SIZE]; char running_mode[SWUPDATE_GENERAL_STRING_SIZE]; char output[SWUPDATE_GENERAL_STRING_SIZE]; diff --git a/parser/parser.c b/parser/parser.c index c2fe175..d3efde5 100644 --- a/parser/parser.c +++ b/parser/parser.c @@ -148,6 +148,17 @@ static bool get_common_fields(parsertype p, void *cfg, struct swupdate_cfg *swcf TRACE("Description %s", swcfg->description); } + if(swcfg->globals.no_state_marker) { + swcfg->bootloader_state_marker = false; + } else { + swcfg->bootloader_state_marker = true; + if((setting = find_node(p, cfg, "bootloader_state_marker", swcfg)) != NULL) { + get_field(p, setting, NULL, &swcfg->bootloader_state_marker); + TRACE("Setting bootloader state marker: %s", + swcfg->bootloader_state_marker == true ? "true" : "false"); + } + } + if(swcfg->globals.no_transaction_marker) { swcfg->bootloader_transaction_marker = false; } else {
Analogously to the bootloader_transaction_marker, introduce a bootloader_state_marker that allows to disable setting ustate. This is useful if the installed update does not require persistent state handling at all like, e.g., applications, or when using a single-copy setup. Signed-off-by: Christian Storm <christian.storm@siemens.com> --- core/stream_interface.c | 8 ++++++-- core/swupdate.c | 8 +++++++- doc/source/sw-description.rst | 13 ++++++++++--- doc/source/swupdate.rst | 3 +++ include/swupdate.h | 2 ++ parser/parser.c | 11 +++++++++++ 6 files changed, 39 insertions(+), 6 deletions(-)