diff mbox series

Errors in image handler on Lua

Message ID CANs53pp81L8-ZDNK8_wXO+ee+mb7qOFMWGf8FMgBod3AKYLCKw@mail.gmail.com
State Changes Requested
Delegated to: Stefano Babic
Headers show
Series Errors in image handler on Lua | expand

Commit Message

Антон Иванов Dec. 23, 2022, 3:51 p.m. UTC
I have implemented an image handler on lua based on the example in
handlers/lua/fpga.lua. However, I am getting the following error:

[ERROR] : SWUPDATE failed [0] ERROR corelib/lua_interface.c :
istream_read_callback : 424 : I/O buffer size is larger than Lua's buffer
size 1024
[ERROR] : SWUPDATE failed [0] ERROR corelib/lua_interface.c : notify_helper
: 719 : Error reading image: Invalid argument

Error comes from *istream_read_callback* function
https://github.com/sbabic/swupdate/blob/2022.05/corelib/lua_interface.c#L424
It seems that this check is not required, because the *luaL_prepbuffsize*
function is called, which will prepare a buffer of the required size.
I tested the code without this check and everything works fine.

Are there hidden reasons to keep this check, or it can be safely removed?

Patch based on version 2022.05:

       lua_pushvalue(L, 2);

Thanks

Comments

Stefano Babic Jan. 12, 2023, 9:06 p.m. UTC | #1
Hi Anton,

On 23.12.22 10:51, Антон Иванов wrote:
> I have implemented an image handler on lua based on the example in 
> handlers/lua/fpga.lua. However, I am getting the following error:
> 
> [ERROR] : SWUPDATE failed [0] ERROR corelib/lua_interface.c : 
> istream_read_callback : 424 : I/O buffer size is larger than Lua's 
> buffer size 1024
> [ERROR] : SWUPDATE failed [0] ERROR corelib/lua_interface.c : 
> notify_helper : 719 : Error reading image: Invalid argument
> 
> Error comes from *istream_read_callback* function 
> https://github.com/sbabic/swupdate/blob/2022.05/corelib/lua_interface.c#L424 <https://github.com/sbabic/swupdate/blob/2022.05/corelib/lua_interface.c#L424>
> It seems that this check is not required, because the 
> *luaL_prepbuffsize* function is called, which will prepare a buffer of 
> the required size.
> I tested the code without this check and everything works fine.
> 
> Are there hidden reasons to keep this check, or it can be safely removed?
> 

I think it can be safely removed.

> Patch based on version 2022.05:
> 
> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
> index dd1be9e..cdbdd59 100644
> --- a/corelib/lua_interface.c
> +++ b/corelib/lua_interface.c
> @@ -420,10 +420,6 @@ copyfile_exit:
> static int istream_read_callback(void *out, const void *buf, size_t len)
> {
>         lua_State* L = (lua_State*)out;
> -       if (len > LUAL_BUFFERSIZE) {
> -               ERROR("I/O buffer size is larger than Lua's buffer size 
> %d", LUAL_BUFFERSIZE);
> -               return -1;
> -       }
> 
>         luaL_checktype(L, 2, LUA_TFUNCTION);
>         lua_pushvalue(L, 2);
> 

Please send a formal patch with your Signed-off-by, so that I could 
merge it - thanks !

Best regards,
Stefano Babic

> Thanks
> 
> -- 
> You received this message because you are subscribed to the Google 
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send 
> an email to swupdate+unsubscribe@googlegroups.com 
> <mailto:swupdate+unsubscribe@googlegroups.com>.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/swupdate/CANs53pp81L8-ZDNK8_wXO%2Bee%2Bmb7qOFMWGf8FMgBod3AKYLCKw%40mail.gmail.com <https://groups.google.com/d/msgid/swupdate/CANs53pp81L8-ZDNK8_wXO%2Bee%2Bmb7qOFMWGf8FMgBod3AKYLCKw%40mail.gmail.com?utm_medium=email&utm_source=footer>.
Storm, Christian Jan. 19, 2023, 12:37 p.m. UTC | #2
Hi,


> > I have implemented an image handler on lua based on the example in
> > handlers/lua/fpga.lua. However, I am getting the following error:
> > 
> > [ERROR] : SWUPDATE failed [0] ERROR corelib/lua_interface.c :
> > istream_read_callback : 424 : I/O buffer size is larger than Lua's buffer
> > size 1024

Hm, what makes you use a larger size or, put another way, what/who chose
the size?

> > [ERROR] : SWUPDATE failed [0] ERROR corelib/lua_interface.c :
> > notify_helper : 719 : Error reading image: Invalid argument
> > 
> > Error comes from *istream_read_callback* function
> > https://github.com/sbabic/swupdate/blob/2022.05/corelib/lua_interface.c#L424 <https://github.com/sbabic/swupdate/blob/2022.05/corelib/lua_interface.c#L424>
> > It seems that this check is not required, because the *luaL_prepbuffsize*
> > function is called, which will prepare a buffer of the required size.
> > I tested the code without this check and everything works fine.
> > 
> > Are there hidden reasons to keep this check, or it can be safely removed?
> > 
> 
> I think it can be safely removed.

That may depend on the Lua version(s) you're using. Did you test all Lua
versions starting with 5.1 and including LuaJIT on x86 as well as on
arm/aarch64?
Note: For Lua 5.1, there's the compat layer in corelib/lua_compat.c wich
implements this functionality as in Lua 5.2.



Kind regards,
   Christian
diff mbox series

Patch

diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
index dd1be9e..cdbdd59 100644
--- a/corelib/lua_interface.c
+++ b/corelib/lua_interface.c
@@ -420,10 +420,6 @@  copyfile_exit:
static int istream_read_callback(void *out, const void *buf, size_t len)
{
       lua_State* L = (lua_State*)out;
-       if (len > LUAL_BUFFERSIZE) {
-               ERROR("I/O buffer size is larger than Lua's buffer size
%d", LUAL_BUFFERSIZE);
-               return -1;
-       }

       luaL_checktype(L, 2, LUA_TFUNCTION);