Skip to content

Commit

Permalink
python310Packages.cffi: patch closures to work on M1 machines
Browse files Browse the repository at this point in the history
Trusts the libffi library inside of nixpkgs on Apple devices.

When Apple's fork of libffi is not detected, cffi assumes that libffi
uses a strategy for creating closures (i.e. callbacks) that is in
certain cases susceptible to a security exploit.

Based on some analysis I did:

  https://groups.google.com/g/python-cffi/c/xU0Usa8dvhk

I believe that libffi already contains the code from Apple's fork that
is deemed safe to trust in cffi.

It uses a more sophisticated strategy for creating trampolines to
support closures that works on Apple Silicon, while the simple approach
that cffi falls back on does not, so this patch enables code that uses
closures on M1 Macs again.

Notably, pyOpenSSL is impacted and will be fixed by this, reported in

  pyca/pyopenssl#873

Note that libffi closures still will not work on signed apps without the
com.apple.security.cs.allow-unsigned-executable-memory entitlement while

  libffi/libffi#621

is still open (which I haven't tested but is my best guess from reading).

I am hopeful that all of these changes will be upstreamed back into cffi
and libffi, and that this comment provides enough breadcrumbs for future
maintainers to track and clean this up.
  • Loading branch information
tjni authored and tm-drtina committed Feb 27, 2024
1 parent be5d150 commit 894a0ca
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
diff -r bac92fcfe4d7 c/_cffi_backend.c
--- a/c/_cffi_backend.c Mon Jul 18 15:58:34 2022 +0200
+++ b/c/_cffi_backend.c Sat Aug 20 12:38:31 2022 -0700
@@ -96,7 +96,7 @@
# define CFFI_CHECK_FFI_PREP_CIF_VAR 0
# define CFFI_CHECK_FFI_PREP_CIF_VAR_MAYBE 0

-#elif defined(__APPLE__) && defined(FFI_AVAILABLE_APPLE)
+#elif defined(__APPLE__)

# define CFFI_CHECK_FFI_CLOSURE_ALLOC __builtin_available(macos 10.15, ios 13, watchos 6, tvos 13, *)
# define CFFI_CHECK_FFI_CLOSURE_ALLOC_MAYBE 1
@@ -6413,7 +6413,7 @@
else
#endif
{
-#if defined(__APPLE__) && defined(FFI_AVAILABLE_APPLE) && !FFI_LEGACY_CLOSURE_API
+#if defined(__APPLE__) && !FFI_LEGACY_CLOSURE_API
PyErr_Format(PyExc_SystemError, "ffi_prep_closure_loc() is missing");
goto error;
#else
16 changes: 13 additions & 3 deletions pkgs/development/python-modules/cffi/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ if isPyPy then null else buildPythonPackage rec {
propagatedBuildInputs = [ pycparser ];

patches =
#
# Trusts the libffi library inside of nixpkgs on Apple devices.
#
# Based on some analysis I did:
#
# https://groups.google.com/g/python-cffi/c/xU0Usa8dvhk
#
# I believe that libffi already contains the code from Apple's fork that is
# deemed safe to trust in cffi.
#

./darwin-use-libffi-closures.diff
# Fix test that failed because python seems to have changed the exception format in the
# final release. This patch should be included in the next version and can be removed when
# it is released.
Expand All @@ -40,9 +52,7 @@ if isPyPy then null else buildPythonPackage rec {
NIX_CFLAGS_COMPILE = lib.optionalString stdenv.cc.isClang
"-Wno-unused-command-line-argument -Wno-unreachable-code -Wno-c++11-narrowing";

# Lots of tests fail on aarch64-darwin due to "Cannot allocate write+execute memory":
# * https://cffi.readthedocs.io/en/latest/using.html#callbacks
doCheck = !stdenv.hostPlatform.isMusl && !(stdenv.isDarwin && stdenv.isAarch64);
doCheck = !stdenv.hostPlatform.isMusl;

checkInputs = [ pytestCheckHook ];

Expand Down
45 changes: 45 additions & 0 deletions pkgs/development/python2-modules/cffi/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{ lib, stdenv, cffi }:

if cffi == null then null else cffi.overridePythonAttrs {
disabledTests = lib.optionals stdenv.isDarwin [
# cannot load library 'c'
"test_FILE"
"test_FILE_object"
"test_FILE_only_for_FILE_arg"
"test_load_and_call_function"
"test_load_library"

# cannot load library 'dl'
"test_dlopen_handle"

# cannot load library 'm'
"test_dir_on_dlopen_lib"
"test_dlclose"
"test_dlopen"
"test_dlopen_constant"
"test_dlopen_flags"
"test_function_typedef"
"test_line_continuation_in_defines"
"test_missing_function"
"test_remove_comments"
"test_remove_line_continuation_comments"
"test_simple"
"test_sin"
"test_sinf"
"test_stdcall_only_on_windows"
"test_wraps_from_stdlib"

# MemoryError
"test_callback_as_function_argument"
"test_callback_crash"
"test_callback_decorator"
"test_callback_large_struct"
"test_callback_returning_void"
"test_cast_functionptr_and_int"
"test_function_pointer"
"test_functionptr_intptr_return"
"test_functionptr_simple"
"test_functionptr_void_return"
"test_functionptr_voidptr_return"
];
}
2 changes: 2 additions & 0 deletions pkgs/top-level/python2-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ with self; with super; {

certifi = callPackage ../development/python2-modules/certifi { };

cffi = callPackage ../development/python2-modules/cffi { inherit cffi; };

chardet = callPackage ../development/python2-modules/chardet { };

cheetah = callPackage ../development/python2-modules/cheetah { };
Expand Down

0 comments on commit 894a0ca

Please sign in to comment.