[PATCH] Lua: Export SWUpdate bootenv get and set functions to Lua scripts

[ste...@herbrechtsmeier.net]

From: Stefan Herbrechtsmeier <stefan.herb...@weidmueller.com>

Signed-off-by: Stefan Herbrechtsmeier <stefan.herb...@weidmueller.com>
---

corelib/lua_interface.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
index 25d84e6..f370474 100644
--- a/corelib/lua_interface.c
+++ b/corelib/lua_interface.c
@@ -16,6 +16,7 @@
#include "lua_util.h"
#include "util.h"
#include "handler.h"
+#include "bootloader.h"

#define LUA_TYPE_PEMBSCR 1
#define LUA_TYPE_HANDLER 2
@@ -648,6 +649,35 @@ static int l_info(lua_State *L) {
return 0;
}

+static int l_get_bootenv(lua_State *L) {
+ const char *name = luaL_checkstring(L, 1);
+ char *value = NULL;
+
+ if (strlen(name))
+ value = bootloader_env_get(name);
+ lua_pop(L, 1);
+
+ lua_pushstring(L, value);
+ free(value);
+
+ return 1;
+}
+
+static int l_set_bootenv(lua_State *L) {
+ const char *name = luaL_checkstring(L, 1);
+ const char *value = luaL_checkstring(L, 2);
+
+ if (strlen(name)) {
+ if (strlen(value))
+ bootloader_env_set(name, value);
+ else
+ bootloader_env_unset(name);
+ }
+ lua_pop(L, 2);
+
+ return 0;
+}
+
#ifdef CONFIG_HANDLER_IN_LUA
static int l_get_tmpdir(lua_State *L)
{
@@ -664,6 +694,8 @@ static const luaL_Reg l_swupdate[] = {
{ "error", l_error },
{ "trace", l_trace },
{ "info", l_info },
+ { "get_bootenv", l_get_bootenv },
+ { "set_bootenv", l_set_bootenv },
{ NULL, NULL }
};

--
2.7.4

[Stefano Babic]

Hi Stefan,

A set() could be very dangerous. I can understand that it could be
helpful, but it is very difficult to track when something is going wronf
and could break the system.

Think about this scenario: a LUA (jandler, script) is accessing to the
environment and changes. Everything fine, after that the next handler
finds an error and aborts the installation. The system remains with the
environment updated by the LUA actor, not with the original - system
does not maybe boot anymore.

This is the reason why u-boot is set at the end of the update: all
handlers ran, and we are sure that upgrading the environment is the last
step. Even if it fails, we have still the old environment.

But if a LUA script can do this, everything happens.

I agree that someone (I read from ML) has the good idea to use fw_setnev
inside shell scripts - another way to try to brick the device.

IMHO this looks dangerous....

Note: l_get_bootenv() can be added without issues, of course.

Best regards,
Stefano

--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================

[Stefan.Herb...@weidmueller.com]

Hi Stefano,

> -----Ursprüngliche Nachricht-----
> Von: Stefano Babic [mailto:sba...@denx.de]
> Gesendet: Mittwoch, 17. Januar 2018 15:48
> An: ste...@herbrechtsmeier.net; swup...@googlegroups.com
> Cc: Herbrechtsmeier Dr.-Ing. , Stefan
> <Stefan.Herb...@weidmueller.com>
> Betreff: Re: [swupdate] [PATCH] Lua: Export SWUpdate bootenv get and set
> functions to Lua scripts

I understand your concerns but the "-e" parameter has also the potential to break your system if you don't restart the system between two updates. Anyway let's keep the risk low if other solution exists.

Alternative I can add support for a Lua hook script to the bootloader environment entries to change the value at parse time and execute the changes at the end of the update. But therefore I need to extend and temporally misuse the img_type to pass the name and value to a Lua handler.

> Note: l_get_bootenv() can be added without issues, of course.

I will resend the patch without the set function.

Best regards

Stefan Herbrechtsmeier
Software Developer Embedded Systems

Weidmüller – Your partner in Industrial Connectivity
We look forward to sharing ideas with you – Let’s connect.

Weidmueller Interface GmbH & Co. KG
Klingenbergstraße 16, 32758 Detmold, Germany
Email: Stefan.Herb...@weidmueller.com – Web: www.weidmueller.com

________________________________
Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 -
Komplementärin: Weidmüller Interface Führungsgesellschaft mbH -
Sitz: Detmold - Amtsgericht Lemgo HRB 3924;
Geschäftsführer: José Carlos Álvarez Tobar, Elke Eckstein, Jörg Timmermann;
USt-ID-Nr. DE124599660

[Stefano Babic]

Hi Stefan,

Not exactly - you are talking about a misuse. SWUpdate is more a
framework, and if the update concept is wrong, there is nothing that can
be done. You example applies just to double-copy, and the designer
should take care that a successful update requires a restart. If the
design is correct, there is no risk.

> Anyway let's keep the risk low if other solution exists.
>
> Alternative I can add support for a Lua hook script to the bootloader environment entries to change the value at parse time and execute the changes at the end of the update.

Maybe we are thinking the same. I thought to have a "set_bootenv" that
handles the bootloader list. The update will be done at the end as
usual. Is this what you mind ?

> But therefore I need to extend and temporally misuse the img_type to pass the name and value to a Lua handler.
>

mmmhh...get_bootenv in LUA returns the environment, and set_bootenv(var,
value) will change just a variable in the list. Not sure whhat you plan
to do.

>> Note: l_get_bootenv() can be added without issues, of course.
>
> I will resend the patch without the set function.

Ok, fine.

[Stefan.Herb...@weidmueller.com]

Hi Stefano,

> -----Ursprüngliche Nachricht-----
> Von: Stefano Babic [mailto:sba...@denx.de]

> Gesendet: Mittwoch, 17. Januar 2018 16:59
> An: Herbrechtsmeier Dr.-Ing. , Stefan
> <Stefan.Herb...@weidmueller.com>; sba...@denx.de;
> ste...@herbrechtsmeier.net; swup...@googlegroups.com
> Betreff: Re: AW: [swupdate] [PATCH] Lua: Export SWUpdate bootenv get

I mean a hook to an embedded-script function in the sw-description:
bootenv: (
{
name = "vram";
value = "4M";
hook = "my_lua_script";
}
);

Do you mean a set_bootenv function which only updates the bootloader environment dictionary? How will you pass the bootloader dictionary to this function?

> > But therefore I need to extend and temporally misuse the img_type to
> pass the name and value to a Lua handler.
> >
>
> mmmhh...get_bootenv in LUA returns the environment, and
> set_bootenv(var,
> value) will change just a variable in the list. Not sure whhat you plan to do.

I need a function to read the current bootloader value. Therefore the get_bootenv should return the live value but the set_bootenv function should postpone the update to the end of the update process.

[Stefano Babic]

Hi Stefan,

This is just a use case, that is when the variables are manipulated at
parse time. We can have LUA scripts, and LUA handlers, too.

>
> Do you mean a set_bootenv function which only updates the bootloader environment dictionary?

Yes, this looks to me more generic. As far as I see, you do not need
this now and it could be postpone until a use case appears.

> How will you pass the bootloader dictionary to this function?

I have not thought to pass the whole dictionary, but I confess I have
not deeply thought how to implement it.

>
>>> But therefore I need to extend and temporally misuse the img_type to
>> pass the name and value to a Lua handler.
>>>
>>
>> mmmhh...get_bootenv in LUA returns the environment, and
>> set_bootenv(var,
>> value) will change just a variable in the list. Not sure whhat you plan to do.
>
> I need a function to read the current bootloader value. Therefore the get_bootenv should return the live value but the set_bootenv function should postpone the update to the end of the update process.
>

Ok

[Stefan.Herb...@weidmueller.com]

Hi Stefano,

> -----Ursprüngliche Nachricht-----
> Von: Stefano Babic [mailto:sba...@denx.de]

> Gesendet: Mittwoch, 17. Januar 2018 18:09

> An: Herbrechtsmeier Dr.-Ing. , Stefan
> <Stefan.Herb...@weidmueller.com>; sba...@denx.de;
> ste...@herbrechtsmeier.net; swup...@googlegroups.com

> Betreff: Re: AW: AW: [swupdate] [PATCH] Lua: Export SWUpdate bootenv

> get and set functions to Lua scripts
>

[snip]

> >>> Alternative I can add support for a Lua hook script to the
> >>> bootloader
> >> environment entries to change the value at parse time and execute the
> >> changes at the end of the update.
> >>
> >> Maybe we are thinking the same. I thought to have a "set_bootenv"
> >> that handles the bootloader list. The update will be done at the end
> >> as usual. Is this what you mind ?
> >
> > I mean a hook to an embedded-script function in the sw-description:
> > bootenv: (
> > {
> > name = "vram";
> > value = "4M";
> > hook = "my_lua_script";
> > }
> > );
>
> This is just a use case, that is when the variables are manipulated at parse
> time. We can have LUA scripts, and LUA handlers, too.
>
> >
> > Do you mean a set_bootenv function which only updates the bootloader
> environment dictionary?
>
> Yes, this looks to me more generic. As far as I see, you do not need this now
> and it could be postpone until a use case appears.

Okay

>
> > How will you pass the bootloader dictionary to this function?
>
> I have not thought to pass the whole dictionary, but I confess I have not
> deeply thought how to implement it.

We could add a pointer of the bootloader environment dictionary to the Lua registry and use the l_set_bootenv function to add a key and value to this dictionary. Therefore we need to pass the dictionary or swupdate_cfg to lua_parser_fn.

> >>> But therefore I need to extend and temporally misuse the img_type to
> >> pass the name and value to a Lua handler.
> >>>
> >>
> >> mmmhh...get_bootenv in LUA returns the environment, and
> >> set_bootenv(var,
> >> value) will change just a variable in the list. Not sure whhat you plan to do.
> >
> > I need a function to read the current bootloader value. Therefore the
> get_bootenv should return the live value but the set_bootenv function
> should postpone the update to the end of the update process.
> >
>
> Ok

What are your naming preferences?
swupdate.get_bootenv("var") or bootloader.getenv("var")?

[Stefano Babic]

Hi Stefan,

On 18/01/2018 08:56, Stefan.Herb...@weidmueller.com wrote:

>>> How will you pass the bootloader dictionary to this function?
>>
>> I have not thought to pass the whole dictionary, but I confess I have not
>> deeply thought how to implement it.
>
> We could add a pointer of the bootloader environment dictionary to the Lua registry and use the l_set_bootenv function to add a key and value to this dictionary. Therefore we need to pass the dictionary or swupdate_cfg to lua_parser_fn.

Yes, something like that, but we have to be sure about the context.
Until everything is running in the installer (=the main process), this
is fine. Anyway, LUA runs in the installer context.

>
>>>>> But therefore I need to extend and temporally misuse the img_type to
>>>> pass the name and value to a Lua handler.
>>>>>
>>>>
>>>> mmmhh...get_bootenv in LUA returns the environment, and
>>>> set_bootenv(var,
>>>> value) will change just a variable in the list. Not sure whhat you plan to do.
>>>
>>> I need a function to read the current bootloader value. Therefore the
>> get_bootenv should return the live value but the set_bootenv function
>> should postpone the update to the end of the update process.
>>>
>>
>> Ok
>
> What are your naming preferences?
> swupdate.get_bootenv("var") or bootloader.getenv("var")?

We have a require("swupdate") and functions from LUA are called with the
"swupdate" prefix. I prefer the first one to have the names consistent.

[ste...@herbrechtsmeier.net]

From: Stefan Herbrechtsmeier <stefan.herb...@weidmueller.com>

Export get_bootenv and set_bootenv functions to the embedded Lua scripts.
The get function reads the bootloader environment variable direct and
the set function postone the write of the bootloader environment variable

to the end of the update process.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herb...@weidmueller.com>

---

Changes in v2:
- Postpone set bootloader environment to the end of the update

corelib/lua_interface.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
include/lua_util.h | 5 +++--
parser/parser.c | 2 +-
3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
index 25d84e6..57f5908 100644

--- a/corelib/lua_interface.c
+++ b/corelib/lua_interface.c
@@ -16,6 +16,7 @@
#include "lua_util.h"
#include "util.h"
#include "handler.h"
+#include "bootloader.h"

#define LUA_TYPE_PEMBSCR 1
#define LUA_TYPE_HANDLER 2

@@ -648,6 +649,36 @@ static int l_info(lua_State *L) {

return 0;
}

+static int l_get_bootenv(lua_State *L) {
+ const char *name = luaL_checkstring(L, 1);
+ char *value = NULL;
+
+ if (strlen(name))
+ value = bootloader_env_get(name);
+ lua_pop(L, 1);
+
+ lua_pushstring(L, value);
+ free(value);
+
+ return 1;
+}
+
+static int l_set_bootenv(lua_State *L) {

+ struct dict *bootenv = (struct dict *)lua_touserdata(L, lua_upvalueindex(1));

+ const char *name = luaL_checkstring(L, 1);
+ const char *value = luaL_checkstring(L, 2);
+
+ if (strlen(name)) {
+ if (strlen(value))

+ dict_set_value(bootenv, name, value);
+ else
+ dict_remove(bootenv, name);

+ }
+ lua_pop(L, 2);
+
+ return 0;
+}
+
#ifdef CONFIG_HANDLER_IN_LUA
static int l_get_tmpdir(lua_State *L)
{

@@ -667,6 +698,12 @@ static const luaL_Reg l_swupdate[] = {
{ NULL, NULL }
};

+static const luaL_Reg l_swupdate_bootenv[] = {

+ { "get_bootenv", l_get_bootenv },
+ { "set_bootenv", l_set_bootenv },

+ { NULL, NULL }
+};
+
#ifdef CONFIG_HANDLER_IN_LUA
static const luaL_Reg l_swupdate_handler[] = {
{ "register_handler", l_register_handler },
@@ -911,17 +948,21 @@ int lua_handlers_init(void)
int lua_handlers_init(void) {return 0;}
#endif

-lua_State *lua_parser_init(const char *buf)
+lua_State *lua_parser_init(const char *buf, struct dict *bootenv)
{
lua_State *L = luaL_newstate(); /* opens Lua */

if (!L)
return NULL;
+
lua_pushlightuserdata(L, (void*)LUA_TYPE_PEMBSCR);
lua_setglobal(L, "SWUPDATE_LUA_TYPE"); /* prime L as LUA_TYPE_PEMBSCR */
luaL_openlibs(L); /* opens the standard libraries */
luaL_requiref(L, "swupdate", luaopen_swupdate, 1 );
+ lua_pushlightuserdata(L, (void *)bootenv);
+ luaL_setfuncs(L, l_swupdate_bootenv, 1);
lua_pop(L, 1); /* remove unused copy left on stack */
+
if (luaL_loadstring(L, buf) || lua_pcall(L, 0, 0, 0)) {
LUAstackDump(L);
ERROR("ERROR preparing Lua embedded script in parser");
diff --git a/include/lua_util.h b/include/lua_util.h
index 479d175..ee5e2cf 100644
--- a/include/lua_util.h
+++ b/include/lua_util.h
@@ -17,7 +17,7 @@

void LUAstackDump (lua_State *L);
int run_lua_script(const char *script, const char *function, char *parms);
-lua_State *lua_parser_init(const char *buf);
+lua_State *lua_parser_init(const char *buf, struct dict *bootenv);
int lua_parser_fn(lua_State *L, const char *fcn, struct img_type *img);
int lua_handlers_init(void);
#define lua_parser_exit(L) lua_close((lua_State *)L)
@@ -63,7 +63,8 @@ void luaL_pushresult(luaL_Buffer_52 *B);

#define lua_State void
#define lua_parser_exit(L)
-static inline lua_State *lua_parser_init(const char __attribute__ ((__unused__)) *buf) { return NULL;}
+static inline lua_State *lua_parser_init(const char __attribute__ ((__unused__)) *buf,
+ struct dict __attribute__ ((__unused__)) *bootenv) { return NULL;}
static inline int lua_parser_fn(lua_State __attribute__ ((__unused__)) *L,
const char __attribute__ ((__unused__)) *fcn,
struct img_type __attribute__ ((__unused__)) *img) { return -1; }
diff --git a/parser/parser.c b/parser/parser.c
index 66a0ba9..06f03b6 100644
--- a/parser/parser.c
+++ b/parser/parser.c
@@ -610,7 +610,7 @@ static int parser(parsertype p, void *cfg, struct swupdate_cfg *swcfg)

if (swcfg->embscript) {
TRACE("Found Lua Software:\n%s\n", swcfg->embscript);
- L = lua_parser_init(swcfg->embscript);
+ L = lua_parser_init(swcfg->embscript, &swcfg->bootloader);
if (!L) {
ERROR("Required embedded script that cannot be loaded");
return -1;
--
2.7.4

[Stefano Babic]

Hi Stefan,

Acked-by: Stefano Babic <sba...@denx.de>

Best regards,
Stefano Babic

[Stefano Babic]

On 23/01/2018 14:52, ste...@herbrechtsmeier.net wrote:

> From: Stefan Herbrechtsmeier <stefan.herb...@weidmueller.com>
>
> Export get_bootenv and set_bootenv functions to the embedded Lua scripts.
> The get function reads the bootloader environment variable direct and
> the set function postone the write of the bootloader environment variable
> to the end of the update process.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herb...@weidmueller.com>
>
> ---
>

Applied to -master, thanks !