Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP_Scripting: patch vendored Lua to 5.3.6 #29116

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tpwrules
Copy link
Contributor

In particular this fixes some exceedingly rare/impossible use-after-frees. The serious ones aren't actually accessible in our scripting engine.

To maintain our alterations, I applied source patches generated from the source repository manually instead of simply overwriting the code. Please see the commit message for full details.

Tested by flying scripting aerobatics on SITL on HW on Cube Orange.

The size overhead is negligible:

Board                    AP_Periph  blimp  bootloader  copter  heli  iofirmware  plane  rover  sub
CubeOrange-periph-heavy  8                 *                                                   
Durandal                            8      *           8       8                 8      8      8
Hitec-Airspeed           *                 *                                                   
KakuteH7-bdshot                     48     *           48      48                48     40     48
MatekF405                           *      *           *       *                 *      *      *
Pixhawk1-1M-bdshot                  *                  *       *                 *      *      *
f103-QiotekPeriph        *                 *                                                   
f303-Universal           *                 *                                                   
iomcu                                                                *                         
revo-mini                           *      *           *       *                 *      *      *
skyviper-v2450                                         *

@IamPete1
Copy link
Member

This is missing a patch to src/Makefile the freebsd case should be:

$(MAKE) $(ALL) SYSCFLAGS="-DLUA_USE_LINUX -I/usr/include/edit" SYSLIBS="-Wl,-E -ledit" CC="cc"

Apart from that this is a exact copy of the 4.5 to 4.6 diff. No idea if everything work but I guess we have to trust the lua maintainers to know what there doing.

@tpwrules
Copy link
Contributor Author

Thanks for the investigation. I didn't catch that makefile; I did the diff based on the Git repo instead of the archives. But I don't think anybody uses our tree to build for freebsd so I'm going to keep it how it is.

For reference, the changes in lparser.c are the only ones I think we have a chance of running into.

@IamPete1
Copy link
Member

Thanks for the investigation. I didn't catch that makefile; I did the diff based on the Git repo instead of the archives. But I don't think anybody uses our tree to build for freebsd so I'm going to keep it how it is.

As far as I can tell the git repo is not a official release, only irregularly mirrored. I used source from: https://www.lua.org/ftp/. I'm sure no one will use the freebsd case, the key thing is to get the diff right so we don't get confused when doing future comparisons. We have the makefile in tree now, so we need to keep it in sync with the the version were using. I'm not sure why the repo does not look the same as the release, it is a bit odd.

We should probably add to the AP_Scripting readme where exactly the source is from.

In particular this fixes some exceedingly rare/impossible
use-after-frees.

Add the new docs from the distribution and clarify where we get our code
from. To maintain our alterations, the following patches have been
applied to the source from upstream's repository (1221e987...75ea9ccb)
to bring the source up to date:

* Bug: Long brackets with a huge number of '=' causes overflow

  A long bracket with too many equal signs can overflow the 'int' used for
  the counting and some arithmetic done on the value. Changing the counter
  to 'size_t' avoids that. (Because what is counted goes to a buffer, an
  overflow in the counter will first raise a buffer-overflow error.)

* Fixed bug in 'lua_upvaluejoin'

  Bug-fix: joining an upvalue with itself could cause a use-after-free
  crash.

* Fixed typos in comments

* Fixed missing GC barriers in compiler and undump

  While building a new prototype, the GC needs barriers for every object
  (strings and nested prototypes) that is attached to the new prototype.

* Updated release number and copyright year

* Fixed bug: invalid mode can crash 'io.popen'

* Fixed bug: Negation overflow in getlocal/setlocal

* 'realloc' can fail when shrinking a block

  According to ISO C, 'realloc' can fail when shrinking a block. If that
  happens, 'l_alloc' simply ignores the fail and returns the original
  block.

* Fixed bug of long strings in binary chunks

  When "undumping" a long string, the function 'LoadVector' can call the
  reader function, which can run the garbage collector, which can collect
  the string being read. So, the string must be anchored during the call
  to 'LoadVector'.
@tpwrules
Copy link
Contributor Author

Added those clarifications, thanks.

@tridge
Copy link
Contributor

tridge commented Jan 28, 2025

@IamPete1 looks good to me, but I'd like to give you a chance to review/approve before merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants