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

NAPI segmentation fault on Arm #3698

Closed
niyan-ly opened this issue Jan 21, 2022 · 5 comments
Closed

NAPI segmentation fault on Arm #3698

niyan-ly opened this issue Jan 21, 2022 · 5 comments

Comments

@niyan-ly
Copy link

Version

v16.13.2

Platform

Darwin YongLins-MacBook-Pro.local 21.2.0 Darwin Kernel Version 21.2.0: Sun Nov 28 20:29:10 PST 2021; root:xnu-8019.61.5~1/RELEASE_ARM64_T8101 arm64

Subsystem

macOS Monterey 12.1

What steps will reproduce the bug?

Background

I ran this test on the m1 macbook, got segmentation fault error. Then I pulled the latest official node docker image, still got same error. It seems more like an arm-build related issue, node-v14 on m1 still got the same error. But it works well on x64 linux(debian 9).

The key point is, if we switch the line code-a and code-b, which means create js_array before get_cb_info, then everything works well.

Step to reproduce

copy these 3 files, code.c test.js binding.gyp.

  1. run node-gyp configure
  2. run node-gyp rebuild
  3. run node test

Result

image

{
  "targets":[
    {
      "target_name": "code",
      "sources": ["code.c"]
    }
  ]
}
// test.js
const addon = require("./build/Debug/code.node");

console.log(addon.return_args('kl'));
// code.c
#include <node_api.h>

napi_value return_args(napi_env env, napi_callback_info info)
{
  napi_value list, binding, new_list;
  size_t len = 4;

  napi_get_cb_info(env, info, &len, &list, &binding, NULL); // code-a
  napi_create_array(env, &new_list); // code-b

  return new_list;
}

napi_value Init(napi_env env, napi_value exports)
{
  napi_value s_return_args, f_return_args;

  napi_create_string_utf8(env, "return_args", NAPI_AUTO_LENGTH, &s_return_args);
  napi_create_function(env, "return_args", NAPI_AUTO_LENGTH, return_args, NULL, &f_return_args);

  napi_set_property(env, exports, s_return_args, f_return_args);
  return NULL;
}

NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)

How often does it reproduce? Is there a required condition?

stable reproducible.

What is the expected behavior?

code should be compile and execute successfully.

What do you see instead?

segmentation fault.
image

Additional information

No response

@benjamingr
Copy link
Member

@nodejs/node-api

@mhdawson
Copy link
Member

@niyan-ly could you get and print out the return code that you see for the a) and b) calls?

@addaleax
Copy link
Member

  napi_value list, binding, new_list;
  size_t len = 4;

  napi_get_cb_info(env, info, &len, &list, &binding, NULL); // code-a

This code is broken regardless of architecture – list is a single value here, but your code indicates that it’s a napi_value array of size 4. This leads to an out-of-bounds write by napi_get_cb_info onto your call stack, and subsequently to the crash. You probably want something closer to

  napi_value list[4];
  napi_value binding, new_list;
  size_t len = 4;

  napi_get_cb_info(env, info, &len, &list, &binding, NULL); // code-a

(To be fair, this is implied in the docs, but not spelled out very explicitly.)

@addaleax addaleax transferred this issue from nodejs/node Jan 21, 2022
mhdawson added a commit to mhdawson/io.js that referenced this issue Jan 21, 2022
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <mdawson@devrus.com>
@mhdawson
Copy link
Member

Create PR to try to clarify - nodejs/node#41635

@niyan-ly
Copy link
Author

@addaleax thank you, I should have read the doc more carefully before open this issue. Anyway, thank you for your detailed explanation.

mhdawson added a commit to mhdawson/io.js that referenced this issue Jan 24, 2022
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <mdawson@devrus.com>
mhdawson added a commit to nodejs/node that referenced this issue Jan 25, 2022
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #41635
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this issue Jan 31, 2022
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#41635
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
ruyadorno pushed a commit to nodejs/node that referenced this issue Feb 8, 2022
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #41635
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams pushed a commit to nodejs/node that referenced this issue Mar 2, 2022
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #41635
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams pushed a commit to nodejs/node that referenced this issue Mar 3, 2022
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #41635
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams pushed a commit to nodejs/node that referenced this issue Mar 14, 2022
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #41635
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
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

No branches or pull requests

4 participants