Re: CVS commit: src/sys/modules/lua

[Rin Okuyama]

On 2021/08/09 7:26, Rin Okuyama wrote:

- Initialize LIST_HEAD.


Hmm, since HEADs are static, this is not necessary. But, it should be
a good practice to initialize HEADs always, IMO...

Thanks,
rin

Re: CVS commit: src/sys/modules/lua

[Alexander Nasonov]

Lourival Pereira Vieira Neto wrote:
 ...
 Index: src/sys/modules/lua/stdlib.h
 diff -u src/sys/modules/lua/stdlib.h:1.1 src/sys/modules/lua/stdlib.h:1.2
 --- src/sys/modules/lua/stdlib.h:1.1  Wed Oct 16 19:44:57 2013
 +++ src/sys/modules/lua/stdlib.h  Sat Jul 19 17:10:02 2014
 @@ -1,7 +1,7 @@
  /*   $NetBSD */
  
  /*
 - * Copyright (c) 2011, Lourival Neto ln...@netbsd.org.
 + * Copyright (c) 2011-2014, Lourival Neto ln...@netbsd.org.
   * All rights reserved.
   *
   * Redistribution and use in source and binary forms, with or without
 @@ -36,11 +36,8 @@
  #include sys/param.h
  #include sys/kmem.h
  
 -#ifndef _LUA_INCLUDE_STDLIB
 -#define _LUA_INCLUDE_STDLIB
 -
 -#define realloc(ptr, nsize)  kmem_alloc(nsize, KM_SLEEP)
 -#define free(ptr)kmem_free(ptr, osize)
 +#ifndef _LUA_INCLUDE_STDLIB_
 +#define _LUA_INCLUDE_STDLIB_
  
  #define exit(EXIT_FAILURE)   return

Infamous hack is still in the tree.


Alex

Re: CVS commit: src/sys/modules/lua

[Lourival Vieira Neto]

Hi Alexander,

On Sat, Jul 19, 2014 at 4:15 PM, Alexander Nasonov al...@yandex.ru wrote:
 Lourival Pereira Vieira Neto wrote:
 ...
 Index: src/sys/modules/lua/stdlib.h
 diff -u src/sys/modules/lua/stdlib.h:1.1 src/sys/modules/lua/stdlib.h:1.2
 --- src/sys/modules/lua/stdlib.h:1.1  Wed Oct 16 19:44:57 2013
 +++ src/sys/modules/lua/stdlib.h  Sat Jul 19 17:10:02 2014
 @@ -1,7 +1,7 @@
  /*   $NetBSD */

  /*
 - * Copyright (c) 2011, Lourival Neto ln...@netbsd.org.
 + * Copyright (c) 2011-2014, Lourival Neto ln...@netbsd.org.
   * All rights reserved.
   *
   * Redistribution and use in source and binary forms, with or without
 @@ -36,11 +36,8 @@
  #include sys/param.h
  #include sys/kmem.h

 -#ifndef _LUA_INCLUDE_STDLIB
 -#define _LUA_INCLUDE_STDLIB
 -
 -#define realloc(ptr, nsize)  kmem_alloc(nsize, KM_SLEEP)
 -#define free(ptr)kmem_free(ptr, osize)
 +#ifndef _LUA_INCLUDE_STDLIB_
 +#define _LUA_INCLUDE_STDLIB_

  #define exit(EXIT_FAILURE)   return

 Infamous hack is still in the tree.

You can always provide a better hack.

Regards,
-- 
Lourival Vieira Neto

Re: CVS commit: src/sys/modules/lua

[Alan Barrett]

On Thu, 31 Oct 2013, Marc Balmer wrote:

Modified Files:
src/sys/modules/lua: Makefile

Log Message:
fix build on arm


Please describe the actual change, not just the reason for it.
Fopr example:

Move -include ${.CURDIR}/luaconf.h from CPPFLAGS to CFLAGS.

--apb (Alan Barrett)

Re: CVS commit: src/sys/modules/lua

[Alexander Nasonov]

Alexander Nasonov wrote:
  +#define exit(EXIT_FAILURE) return
 
 You only need to make a change in one place in ldo.c:
 
 @@ -105,7 +110,11 @@ void luaD_throw (lua_State *L, int errco
lua_unlock(L);
G(L)-panic(L);
  }
 +#if defined(_KERNEL)
 +panic(luaD_throw(), errcode=%d, errcode);
 +#else
  exit(EXIT_FAILURE);
 +#endif
}
  }

I'm having a private discussion with Chris Badura about the above panic
call.

He made me realise that I assume that Lua kernel framework always
executes scripts in a protected environment (pcall or xpcall) but I
don't actually know whether it's the case. If it's not, than Lua script
can indeed panic the kernel.

On the other hand, ignoring it and continuing as if nothing happened is
even crazier idea.

We just need to make sure that all entry points to Lua are protected and
hope that the above panic will never trigger.

Alex

Re: CVS commit: src/sys/modules/lua

[Christoph Badura]

On Tue, Oct 22, 2013 at 09:25:19AM +0100, Alexander Nasonov wrote:
 We just need to make sure that all entry points to Lua are protected and
 hope that the above panic will never trigger.

Actually, I would prefer if that call to panic wasn't there at all.
Instead the script/state should be aborted noisily.

--chris

Re: CVS commit: src/sys/modules/lua

[Alexander Nasonov]

Christoph Badura wrote:
 On Tue, Oct 22, 2013 at 09:25:19AM +0100, Alexander Nasonov wrote:
  We just need to make sure that all entry points to Lua are protected and
  hope that the above panic will never trigger.
 
 Actually, I would prefer if that call to panic wasn't there at all.
 Instead the script/state should be aborted noisily.

I have a simple code you can play with in userspace.

#include lua.h
#include lauxlib.h

const char prog[] =
print'hi'\n
error'throw'\n
print'dead code'\n;

int main()
{
lua_State *L;

L = luaL_newstate();
luaL_openlibs(L);

if (luaL_loadstring(L, prog) == 0) {
// Dangerous:
lua_call(L, 0, 0); // no args and no return values
// Protected call should be safe:
//lua_pcall(L, 0, 0, 0);
}

lua_close(L);
return 0;
}

If you link it with vanilla Lua, you'll get 

$ gcc -I/usr/pkg/include/lua-5.1/ -O -g lua-throw.c  -L/usr/pkg/lib 
-Wl,-rpath,/usr/pkg/lib  -llua5.1
$ ./a.out
hi
PANIC: unprotected error in call to Lua API ([string print'hi'...]:2: throw)

If you comment out exit(EXIT_FAILURE) in luaD_throw(), you'll get a
crash because Lua will try to execute the third line while its state
is inconsistent:

$ gcc -I/usr/pkg/include/lua-5.1/ -O -g lua-throw.c  -L `pwd`/lua-5.1.5/src/  
-llua -lm
$ gdb ./a.out
(gdb) run
Starting program: /home/alnsn/src/test/a.out
hi
PANIC: unprotected error in call to Lua API ([string print'hi'...]:2: throw)

Program received signal SIGSEGV, Segmentation fault.
0xb7d8 in luaD_precall ()
(gdb) bt
#0  0xb7d8 in luaD_precall ()
#1  0x00011084 in luaV_execute ()
#2  0x in ?? ()

You really need this panic or KASSERT even if you make sure all your
scripts are properly isolated. You can achieve these in two ways:

1. Set a panic handler with lua_atpanic() which jumps to your safety
   point (if your handler returns, the control is passed to the line
   in question).
2. Make sure that all scripts are executed using lua_pcall. For
   instance, code that loads kmods written in Lua can do this
   seamlessly.


While I agree that it's good to have a protection from fool scripts, but
being able to control loading of scripts manually have advantages too.

The link below is a skeleton for bpfjit generator. It doesn't yet
generate a real code but it creates a Lua array of instructions, passes
it from C to Lua, creates sljit compiler object and gerenates a simple
function inside Lua script, then returns that object to C where it's
casted to C object. If you look at interface, you won't see Lua at all,
it's hidden from public.

https://github.com/alnsn/luaSljit/blob/master/examples/bpfjit/bpfjit.c

Alex

Re: CVS commit: src/sys/modules/lua

[Marc Balmer]

Am 22.10.13 14:24, schrieb Christoph Badura:
 On Tue, Oct 22, 2013 at 09:25:19AM +0100, Alexander Nasonov wrote:
 We just need to make sure that all entry points to Lua are protected and
 hope that the above panic will never trigger.
 
 Actually, I would prefer if that call to panic wasn't there at all.
 Instead the script/state should be aborted noisily.
I absolutely agree.  A problem within a Lua state must not cause a
kernel panic.  The only way to panic from Lua should be calling
core.panic() explicitely.

Re: CVS commit: src/sys/modules/lua

[Alexander Nasonov]

Marc Balmer wrote:
 Module Name:  src
 Committed By: mbalmer
 Date: Wed Oct 16 19:44:58 UTC 2013
 
 Added Files:
   src/sys/modules/lua: Makefile assert.h ctype.h errno.h infinite.lua
   inttypes.h limits.h locale.h lua.c luaconf.h luavar.h math.h
   setjmp.h signal.h stdarg.h stddef.h stdio.h stdlib.h string.h
   test.lua
 
 Log Message:
 welcome lua(4), a devide driver that can create and control Lua states inside 
 the kernel
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r0 -r1.1 src/sys/modules/lua/Makefile \
 src/sys/modules/lua/assert.h src/sys/modules/lua/ctype.h \
 src/sys/modules/lua/errno.h src/sys/modules/lua/infinite.lua \
 src/sys/modules/lua/inttypes.h src/sys/modules/lua/limits.h \
 src/sys/modules/lua/locale.h src/sys/modules/lua/lua.c \
 src/sys/modules/lua/luaconf.h src/sys/modules/lua/luavar.h \
 src/sys/modules/lua/math.h src/sys/modules/lua/setjmp.h \
 src/sys/modules/lua/signal.h src/sys/modules/lua/stdarg.h \
 src/sys/modules/lua/stddef.h src/sys/modules/lua/stdio.h \
 src/sys/modules/lua/stdlib.h src/sys/modules/lua/string.h \
 src/sys/modules/lua/test.lua
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.
 

 Added files:
 
 Index: src/sys/modules/lua/Makefile
 diff -u /dev/null src/sys/modules/lua/Makefile:1.1
 --- /dev/null Wed Oct 16 19:44:58 2013
 +# Compatability code
 +SRCS+=   strcspn.c \
 + strncat.c \
 + strpbrk.c \
 + strspn.c

strcspn searches for \n\r when reporting a chunk name in error
handler. We can assume that no kernel scripts are MSDOS files and use
strchr. strncat in the same function. I'm suprised that it's not in the
kernel already. I don't see strspn being used anywhere.

 Index: src/sys/modules/lua/lua.c

I didn't review this file at all.

 Index: src/sys/modules/lua/luaconf.h
 diff -u /dev/null src/sys/modules/lua/luaconf.h:1.1
 --- /dev/null Wed Oct 16 19:44:58 2013
 +++ src/sys/modules/lua/luaconf.h Wed Oct 16 19:44:57 2013

This file is interesing and I have several comments.

 +#ifndef LUA_CORE
 +#define LUA_CORE
 +#endif

It's not a good idea to define LUA_CORE in luaconf.h file. All Lua core
.c files define LUA_CORE before including luaconf.h. For all other files
it should be undefined.
If you're trying add a new file to a set of Lua core files, you can
define LUA_CORE the same way as other core files do.

 +@@ LUA_INTEGER is the integral type used by lua_pushinteger/lua_tointeger.
 +** CHANGE that if ptrdiff_t is not adequate on your machine. (On most
 +** machines, ptrdiff_t gives a good choice between int or long.)
 +*/
 +#define LUA_INTEGER  ptrdiff_t

It's better to use the same long long type here.

 +#if defined(LUA_USE_READLINE)
 +#include stdio.h
 +#include readline/readline.h
 +#include readline/history.h
 +#define lua_readline(L,b,p)  ((void)L, ((b)=readline(p)) != NULL)
 +#define lua_saveline(L,idx) \
 + if (lua_strlen(L,idx)  0)  /* non-empty line? */ \
 +   add_history(lua_tostring(L, idx));  /* add it to history */
 +#define lua_freeline(L,b)((void)L, free(b))
 +#else

You can probably change the above #else to #elif !defined(_KERNEL).
It may help you in getting rid of fputs and fflush stubs.

 +#define lua_readline(L,b,p)  \
 + ((void)L, fputs(p, stdout), fflush(stdout),  /* show prompt */ \
 + fgets(b, LUA_MAXINPUT, stdin) != NULL)  /* get line */
 +#define lua_saveline(L,idx)  { (void)L; (void)idx; }
 +#define lua_freeline(L,b){ (void)L; (void)b; }
 +#endif
 +#if LUAI_BITSINT = 32
 +#define LUAI_UINT32  unsigned int
 +#define LUAI_INT32   int
 +#define LUAI_MAXINT32INT_MAX
 +#define LUAI_UMEMsize_t
 +#define LUAI_MEM ptrdiff_t

Because ptrdiff_t is undefined, it's better to use uintptr_t/intptr_t
for LUAI_UMEM/LUAI_MEM.

 +#define LUAL_BUFFERSIZE  BUFSIZ

It's easier to use ifdef _KERNEL rather than defining BUFSZ in your
stdio.h stub file.

 +#ifdef _KERNEL
You need to

#define LUA_NUMBER_LONGLONG

(see also LUA_USELONGLONG below, I'm not yet quite sure whether it's
related)

 +#define LUA_NUMBER   long long
 +
 +#else
 +#define LUA_NUMBER_DOUBLE
 +#define LUA_NUMBER   double
 +#endif
 +#if defined(LUA_CORE)
 +#ifdef _KERNEL
 +#define luai_nummod(a,b) ((a)%(b))
 +#define luai_numpow(a,b) luai_nummul(a,b)

a*b != pow(a,b)

 +#if defined(LUA_USELONGLONG)

You probably want to switch to ll in the kernel.

 +#undef LUA_CORE

See above.

 +++ src/sys/modules/lua/stddef.h  Wed Oct 16 19:44:57 2013
 @@ -0,0 +1,16 @@
 +/*   $NetBSD */
 +
 +/*
 + * This file is a placeholder only, to allow Lua to be compiled from
 + * unchanged sources.
 + */
 +
 +#include sys/types.h
 +
 +#ifdef  _BSD_PTRDIFF_T_
 +typedef _BSD_PTRDIFF_T_  ptrdiff_t;
 +#undef  _BSD_PTRDIFF_T_
 +#endif

ptrdiff_t isn't defined in the kernel and I bet there is a reason why
it's undefined. Unfortuntely, 

Re: CVS commit: src/sys/modules/lua

[Alexander Nasonov]

Marc Balmer wrote:
 Module Name:  src
 Committed By: mbalmer
 Date: Wed Oct 16 19:44:58 UTC 2013
 
 Added Files:
   src/sys/modules/lua: Makefile assert.h ctype.h errno.h infinite.lua
   inttypes.h limits.h locale.h lua.c luaconf.h luavar.h math.h
   setjmp.h signal.h stdarg.h stddef.h stdio.h stdlib.h string.h
   test.lua
 
 Log Message:
 welcome lua(4), a devide driver that can create and control Lua states inside 
 the kernel
 

First of all, the code has been imported the wrong way as I stated in
the previous email. You should properly import Lua 5.2 to sys/external/
and then apply local changes required to run it in kernel space.

Second, lua device driver has a limited application. We need a generic
lua module. Your lua device driver can then use it.
For example, if I want to generate bpf code when a user sends a filter
program to /dev/bpf* with Lua, I don't need a device driver. I need to
open a regular lua_State, load sljit module, run bfjit.lua script and
close the state afterwards.

Also, I have a couple of small things.

1. Why do you use lua_ prefix for private functions? You get very close
to lua names: lua_reader vs lua_Reader of lua_require vs luaL_requiref.

2. When you call lua_tostring() you often (or never???) don't check
whether it returns NULL. You can easily panic the kernel when you try
to print a table.

Alex

Re: CVS commit: src/sys/modules/lua

[Marc Balmer]

Am 18.10.13 09:16, schrieb Alexander Nasonov:
 Marc Balmer wrote:
 Module Name: src
 Committed By:mbalmer
 Date:Wed Oct 16 19:44:58 UTC 2013

 Added Files:
  src/sys/modules/lua: Makefile assert.h ctype.h errno.h infinite.lua
  inttypes.h limits.h locale.h lua.c luaconf.h luavar.h math.h
  setjmp.h signal.h stdarg.h stddef.h stdio.h stdlib.h string.h
  test.lua

 Log Message:
 welcome lua(4), a devide driver that can create and control Lua states 
 inside the kernel

 
 First of all, the code has been imported the wrong way as I stated in
 the previous email. You should properly import Lua 5.2 to sys/external/
 and then apply local changes required to run it in kernel space.

This was not an import of Lua.  The Lua sources already are in the tree.
 When we are ready to move from Lua 5.1 to Lua 5.2, Lua 5.2 will be
imported to sys/external, as I already mentioned.

 Second, lua device driver has a limited application. We need a generic
 lua module. Your lua device driver can then use it.

This is generic.  The device driver is only there to control Lua states,
load code into it, provide an ioctl interface to userland applications.
 When code in the kernel needs a Lua state, it does not use the lua(4)
driver to create one (but the lua kernel module must be loaded, of course).

 For example, if I want to generate bpf code when a user sends a filter
 program to /dev/bpf* with Lua, I don't need a device driver. I need to
 open a regular lua_State, load sljit module, run bfjit.lua script and
 close the state afterwards.

Which you can already do.  I will have an example for that later.  The
function prototypes ar in sys/sys/lua.h, btw.

 Also, I have a couple of small things.
 
 1. Why do you use lua_ prefix for private functions? You get very close
 to lua names: lua_reader vs lua_Reader of lua_require vs luaL_requiref.

It does not really matter as long as the names don't clash.

 2. When you call lua_tostring() you often (or never???) don't check
 whether it returns NULL. You can easily panic the kernel when you try
 to print a table.

I will look into this.  The return values should be checked, for sure.

Thanks for your feedback,
Marc

Re: CVS commit: src/sys/modules/lua

[Jukka Ruohonen]

On Fri, Oct 18, 2013 at 08:16:32AM +0100, Alexander Nasonov wrote:
 First of all, the code has been imported the wrong way as I stated in
 the previous email. You should properly import Lua 5.2 to sys/external/
 and then apply local changes required to run it in kernel space.

I agree. Already for the sake of consistency and sanity; if one looks at at
sys/modules, there is no existing code in it.

On Fri, Oct 18, 2013 at 09:29:44AM +0200, Marc Balmer wrote:
 This was not an import of Lua.  The Lua sources already are in the tree.
  When we are ready to move from Lua 5.1 to Lua 5.2, Lua 5.2 will be
 imported to sys/external, as I already mentioned.

Why not do things right the first time? Sounds a like a recipe for a mess.

- Jukka.

Re: CVS commit: src/sys/modules/lua

[Taylor R Campbell]

   From: Marc Balmer mbal...@netbsd.org
   Date: Wed, 16 Oct 2013 19:44:58 +

   welcome lua(4), a devide driver that can create and control Lua states 
inside 
   the kernel

Did you get core approval for this?  The public discussion is still
ongoing, and still lacking in evidence, and there has been no public
statement by core as far as I am aware, nor did any members of core I
asked have any recollection of approving this.  Please don't steamroll
over public discussions like this.

Re: CVS commit: src/sys/modules/lua

[Marc Balmer]

Am 18.10.13 14:24, schrieb Taylor R Campbell:
From: Marc Balmer mbal...@netbsd.org
Date: Wed, 16 Oct 2013 19:44:58 +
 
welcome lua(4), a devide driver that can create and control Lua states 
 inside 
the kernel
 
 Did you get core approval for this?  The public discussion is still
 ongoing, and still lacking in evidence, and there has been no public
 statement by core as far as I am aware, nor did any members of core I
 asked have any recollection of approving this.  Please don't steamroll
 over public discussions like this.
Just for the record, yes, I got core's approval.

Re: CVS commit: src/sys/modules/lua

[Alexander Nasonov]

Marc Balmer wrote:
 This was not an import of Lua.  The Lua sources already are in the tree.
  When we are ready to move from Lua 5.1 to Lua 5.2, Lua 5.2 will be
 imported to sys/external, as I already mentioned.

In this case, kernel changes should have been added to 5.1 source and
later ported to 5.2 code.

  For example, if I want to generate bpf code when a user sends a filter
  program to /dev/bpf* with Lua, I don't need a device driver. I need to
  open a regular lua_State, load sljit module, run bfjit.lua script and
  close the state afterwards.
 
 Which you can already do.  I will have an example for that later.  The
 function prototypes ar in sys/sys/lua.h, btw.

ok

 It does not really matter as long as the names don't clash.

It potentially makes upgrading to a newer version of Lua more difficult
because your private names may conflict with the new version of Lua API.
It also harder to read code.

Alex

Re: CVS commit: src/sys/modules/lua

[Marc Balmer]

Am 18.10.13 19:13, schrieb Alexander Nasonov:
 Marc Balmer wrote:
 This was not an import of Lua.  The Lua sources already are in the tree.
  When we are ready to move from Lua 5.1 to Lua 5.2, Lua 5.2 will be
 imported to sys/external, as I already mentioned.
 
 In this case, kernel changes should have been added to 5.1 source and
 later ported to 5.2 code.

I don't understand you here.  The Lua codes wher imported unaltered.

 
 For example, if I want to generate bpf code when a user sends a filter
 program to /dev/bpf* with Lua, I don't need a device driver. I need to
 open a regular lua_State, load sljit module, run bfjit.lua script and
 close the state afterwards.

 Which you can already do.  I will have an example for that later.  The
 function prototypes ar in sys/sys/lua.h, btw.
 
 ok
 
 It does not really matter as long as the names don't clash.
 
 It potentially makes upgrading to a newer version of Lua more difficult
 because your private names may conflict with the new version of Lua API.

That would be spotted immediately when doing a test build.

 It also harder to read code.
That is only a matter of taste, imo.

Re: CVS commit: src/sys/modules/lua

[Alan Barrett]

On Fri, 18 Oct 2013, Marc Balmer wrote:
Did you get core approval for this?  The public discussion is 
still ongoing, and still lacking in evidence, and there has 
been no public statement by core as far as I am aware, nor did 
any members of core I asked have any recollection of approving 
this.  Please don't steamroll over public discussions like 
this.

Just for the record, yes, I got core's approval.


Could you tell us approximately when you got that approval?  It 
wasn't in the past two years while I have been a member of core.


--apb (Alan Barrett)

Re: CVS commit: src/sys/modules/lua

[Izumi Tsutsui]

apb@ wrote:

 On Fri, 18 Oct 2013, Marc Balmer wrote:
  Did you get core approval for this?  The public discussion is 
  still ongoing, and still lacking in evidence, and there has 
  been no public statement by core as far as I am aware, nor did 
  any members of core I asked have any recollection of approving 
  this.  Please don't steamroll over public discussions like 
  this.
  Just for the record, yes, I got core's approval.
 
 Could you tell us approximately when you got that approval?  It 
 wasn't in the past two years while I have been a member of core.

Lua was initially imported ~three years ago
http://www.netbsd.org/changes/changes-6.0.html#lua%281%29
as a result of GSoC 2010:
http://mail-index.netbsd.org/tech-kern/2010/10/05/msg008900.html
http://netbsd-soc.sourceforge.net/projects/luakern/

IMO, importing lua is a good material for advocacy
(there are so many mentions about it on Twitter/Facebook etc.)
while it took too long..
---
Izumi Tsutsui

Re: CVS commit: src/sys/modules/lua

[Marc Balmer]

Am 17.10.13 00:26, schrieb Alexander Nasonov:
 Two things attracted my eyes after a very quick look.
 
 +#define exit(EXIT_FAILURE) return
 
 I can't believe this code in the tree! I thought we agreed that it will
 be changed.
 
 What's the point of compiling unmodified Lua sources if it forces you
 to write ugly hacks? It's perfectly fine to insert few #ifndef _KERNEL
 to the Lua source.

Agreed, I think.

 
 +++ src/sys/modules/lua/infinite.lua Wed Oct 16 19:44:57 2013
 @@ -0,0 +1,2 @@
 +while (1) do end
 
 Why do you need this file at all and why the code starts like C and ends
 like Lua?
 
   while true do end
 
 no?

It is Lua code.  It is an infinite loop.  It is there so that execution
limit count can be tested.