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.

Sign up for GitHub

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

Jump to bottom

build: Make libffi closure optional #9

Merged
merged 2 commits into from
Jan 24, 2017
Merged

build: Make libffi closure optional #9

merged 2 commits into from
Jan 24, 2017

Conversation

ueno
Copy link
Member

@ueno ueno commented Nov 24, 2016

libffi's closure support is not available on all platforms and may fail
at run time if running under a stricter SELinux policy. Fallback to
pre-compiled closures if it is not usable.

This depends on #8.

@ueno ueno force-pushed the wip/dueno/libffi-optional branch 2 times, most recently from dac7fa5 to cc09685 Compare November 24, 2016 15:43
@ueno ueno added this to the 0.23.3 milestone Nov 28, 2016
@nmav
Copy link
Contributor

nmav commented Nov 28, 2016

A question I had is what should happen when the limit is reached? I've extended test-managed to cover for that, but I reach an assert() inside the library. Is that expected, or am I doing something wrong?
limit-test.txt

nmav
nmav reviewed Nov 28, 2016
View reviewed changes
Copy link
Contributor

@nmav nmav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok, except for the points raised. I am not sure if these are valid issues though.

configure.ac
with_libffi="yes"
fi

AM_CONDITIONAL(WITH_FFI, test "$with_libffi" = "yes")

AC_ARG_WITH([fixed-closure],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment on the name --with-fixed-closure. The term closure may be a bit cryptic for the one compiling. A suggestion could be to use the term --with-max-managed-modules or something similar that will be more clear on the intention, by using terms from the manual.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, changed to --with-max-managed-modules.

p11-kit/virtual.c.template
@@ -46,7 +46,7 @@
#include <stdlib.h>
#include <string.h>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely the copyright info of that file needs to be updated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

p11-kit/virtual.c.template
@@ -327,87 +409,142 @@ p11_virtual_wrap (p11_virtual *virt,
wrapper->destroyer = destroyer;
wrapper->bound.version.major = CRYPTOKI_VERSION_MAJOR;
wrapper->bound.version.minor = CRYPTOKI_VERSION_MINOR;
wrapper->fixed_index = -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if first call to to p11_virtual_wrap() goes via the ffi interface, while the second fails, or vice-versa? I think in that case the p11_virtual_wrap() will reset the index.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not the case, since p11_virtual_wrap() always allocates a new wrapper instance.

configure.ac
[fixed_closure=64])

AC_DEFINE_UNQUOTED(P11_VIRTUAL_MAX_FIXED, [$fixed_closure], [The number of fallback closures compiled in])
AC_SUBST(fixed_closure)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to print that max_fixed value at the end with the rest of parameters.

@nmav
Copy link
Contributor

nmav commented Nov 28, 2016

The gnutls test suite passes with the changes above.

@ueno
Copy link
Member Author

ueno commented Nov 30, 2016

Thanks for the review. I have updated the patches reflecting the comments, and incorporated the test into this PR.

@ueno ueno modified the milestones: future, 0.23.3 Dec 6, 2016
ueno and others added 2 commits January 24, 2017 12:23
@ueno
build: Make libffi closure optional
d17f3e2 
libffi's closure support is not available on all platforms and may fail
at run time if running under a stricter SELinux policy.  Fallback to
pre-compiled closures if it is not usable.

https://bugs.freedesktop.org/show_bug.cgi?id=97611
@ueno
test: Check exhaustion of fixed closures
969237f
@ueno
Copy link
Member Author

ueno commented Jan 24, 2017

As I abandoned #8, I rewrote this without the generation script.

@ueno ueno merged commit c01b59e into p11-glue:master Jan 24, 2017
@ueno ueno deleted the wip/dueno/libffi-optional branch January 24, 2017 13:42
@nora-pxh nora-pxh mentioned this pull request Dec 27, 2021
Open
neverpanic added a commit to neverpanic/p11-kit that referenced this pull request Feb 1, 2023
@neverpanic
rpc: Fix two off-by-one errors identified by asan
c151edc 
Reads after the end of the array happen when removing elements as well
as if the last element was removed. Note that the new if is required
because memmove() expects a size_t as length, so we must ensure that it
can not be negative.

The full AddressSanitizer report is:

| =================================================================
| ==27174==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x616000000bb0 at pc 0x00010f8b618d bp 0x7ff7b0b5c3c0 sp 0x7ff7b0b5bb80
| READ of size 560 at 0x616000000bb0 thread T0
|     #0 0x10f8b618c in wrap_memcpy+0x16c (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1f18c)
|     p11-glue#1 0x111a9bac5 in rpc_C_GetMechanismList+0x18c (p11-kit-client.so:x86_64+0x18ac5)
|     p11-glue#2 0x111823534 in p11prov_GetMechanismList interface.gen.c:138
|     p11-glue#3 0x11184e404 in get_slot_mechanisms session.c:113
|     p11-glue#4 0x11184c3ba in p11prov_init_slots session.c:226
|     p11-glue#5 0x111843fed in p11prov_module_init provider.c:1035
|     p11-glue#6 0x1118417a9 in OSSL_provider_init provider.c:1102
|     p11-glue#7 0x11080ee97 in provider_activate+0x117 (libcrypto.3.dylib:x86_64+0x12be97)
|     p11-glue#8 0x11080eced in ossl_provider_activate+0x42 (libcrypto.3.dylib:x86_64+0x12bced)
|     p11-glue#9 0x11080db4a in provider_conf_init+0x2a2 (libcrypto.3.dylib:x86_64+0x12ab4a)
|     p11-glue#10 0x11075ae36 in CONF_modules_load+0x37b (libcrypto.3.dylib:x86_64+0x77e36)
|     p11-glue#11 0x11075b0c9 in CONF_modules_load_file_ex+0x78 (libcrypto.3.dylib:x86_64+0x780c9)
|     p11-glue#12 0x11075b899 in ossl_config_int+0x36 (libcrypto.3.dylib:x86_64+0x78899)
|     p11-glue#13 0x110806ac9 in ossl_init_config_ossl_+0xa (libcrypto.3.dylib:x86_64+0x123ac9)
|     p11-glue#14 0x7ff818b4acc4 in __pthread_once_handler+0x40 (libsystem_pthread.dylib:x86_64+0x2cc4)
|     p11-glue#15 0x7ff818b61725 in _os_once_callout+0x11 (libsystem_platform.dylib:x86_64+0x1725)
|     p11-glue#16 0x7ff818b4ac72 in pthread_once+0x49 (libsystem_pthread.dylib:x86_64+0x2c72)
|     p11-glue#17 0x11081126c in CRYPTO_THREAD_run_once+0x8 (libcrypto.3.dylib:x86_64+0x12e26c)
|     p11-glue#18 0x11080691d in OPENSSL_init_crypto+0x446 (libcrypto.3.dylib:x86_64+0x12391d)
|     p11-glue#19 0x10f6032dc in OPENSSL_init_ssl+0x79 (libssl.3.dylib:x86_64+0x102dc)
|     p11-glue#20 0x10f3c6287 in main+0x62 (openssl:x86_64+0x100024287)
|     p11-glue#21 0x115f5252d in start+0x1cd (dyld:x86_64+0x552d)
|
| 0x616000000bb0 is located 0 bytes to the right of 560-byte region [0x616000000980,0x616000000bb0)
| allocated by thread T0 here:
|     #0 0x10f8e1ed0 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4aed0)
|     p11-glue#1 0x111f1432f in CRYPTO_malloc+0x8f (libcrypto.3.dylib:x86_64+0x16332f)
|     p11-glue#2 0x11184e1a8 in get_slot_mechanisms session.c:107
|     p11-glue#3 0x11184c3ba in p11prov_init_slots session.c:226
|     p11-glue#4 0x111843fed in p11prov_module_init provider.c:1035
|     p11-glue#5 0x1118417a9 in OSSL_provider_init provider.c:1102
|     p11-glue#6 0x11080ee97 in provider_activate+0x117 (libcrypto.3.dylib:x86_64+0x12be97)
|     p11-glue#7 0x11080eced in ossl_provider_activate+0x42 (libcrypto.3.dylib:x86_64+0x12bced)
|     p11-glue#8 0x11080db4a in provider_conf_init+0x2a2 (libcrypto.3.dylib:x86_64+0x12ab4a)
|     p11-glue#9 0x11075ae36 in CONF_modules_load+0x37b (libcrypto.3.dylib:x86_64+0x77e36)
|     p11-glue#10 0x11075b0c9 in CONF_modules_load_file_ex+0x78 (libcrypto.3.dylib:x86_64+0x780c9)
|     p11-glue#11 0x11075b899 in ossl_config_int+0x36 (libcrypto.3.dylib:x86_64+0x78899)
|     p11-glue#12 0x110806ac9 in ossl_init_config_ossl_+0xa (libcrypto.3.dylib:x86_64+0x123ac9)
|     p11-glue#13 0x7ff818b4acc4 in __pthread_once_handler+0x40 (libsystem_pthread.dylib:x86_64+0x2cc4)
|     p11-glue#14 0x7ff818b61725 in _os_once_callout+0x11 (libsystem_platform.dylib:x86_64+0x1725)
|     p11-glue#15 0x7ff818b4ac72 in pthread_once+0x49 (libsystem_pthread.dylib:x86_64+0x2c72)
|     p11-glue#16 0x11081126c in CRYPTO_THREAD_run_once+0x8 (libcrypto.3.dylib:x86_64+0x12e26c)
|     p11-glue#17 0x11080691d in OPENSSL_init_crypto+0x446 (libcrypto.3.dylib:x86_64+0x12391d)
|     p11-glue#18 0x10f6032dc in OPENSSL_init_ssl+0x79 (libssl.3.dylib:x86_64+0x102dc)
|     p11-glue#19 0x10f3c6287 in main+0x62 (openssl:x86_64+0x100024287)
|     p11-glue#20 0x115f5252d in start+0x1cd (dyld:x86_64+0x552d)
|
| SUMMARY: AddressSanitizer: heap-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1f18c) in wrap_memcpy+0x16c
| Shadow bytes around the buggy address:
|   0x1c2c00000120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x1c2c00000130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| =>0x1c2c00000170: 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa
|   0x1c2c00000180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x1c2c00000190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| Shadow byte legend (one shadow byte represents 8 application bytes):
|   Addressable:           00
|   Partially addressable: 01 02 03 04 05 06 07
|   Heap left redzone:       fa
|   Freed heap region:       fd
|   Stack left redzone:      f1
|   Stack mid redzone:       f2
|   Stack right redzone:     f3
|   Stack after return:      f5
|   Stack use after scope:   f8
|   Global redzone:          f9
|   Global init order:       f6
|   Poisoned by user:        f7
|   Container overflow:      fc
|   Array cookie:            ac
|   Intra object redzone:    bb
|   ASan internal:           fe
|   Left alloca redzone:     ca
|   Right alloca redzone:    cb
| ==27174==ABORTING
| Abort trap: 6

Signed-off-by: Clemens Lang <[email protected]>
@neverpanic neverpanic mentioned this pull request Feb 1, 2023
Closed
ueno pushed a commit to ueno/p11-kit that referenced this pull request Feb 28, 2023
@neverpanic @ueno
rpc: Fix two off-by-one errors identified by asan
a25a24c 
Reads after the end of the array happen when removing elements as well
as if the last element was removed. Note that the new if is required
because memmove() expects a size_t as length, so we must ensure that it
can not be negative.

The full AddressSanitizer report is:

| =================================================================
| ==27174==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x616000000bb0 at pc 0x00010f8b618d bp 0x7ff7b0b5c3c0 sp 0x7ff7b0b5bb80
| READ of size 560 at 0x616000000bb0 thread T0
|     #0 0x10f8b618c in wrap_memcpy+0x16c (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1f18c)
|     p11-glue#1 0x111a9bac5 in rpc_C_GetMechanismList+0x18c (p11-kit-client.so:x86_64+0x18ac5)
|     p11-glue#2 0x111823534 in p11prov_GetMechanismList interface.gen.c:138
|     p11-glue#3 0x11184e404 in get_slot_mechanisms session.c:113
|     p11-glue#4 0x11184c3ba in p11prov_init_slots session.c:226
|     p11-glue#5 0x111843fed in p11prov_module_init provider.c:1035
|     p11-glue#6 0x1118417a9 in OSSL_provider_init provider.c:1102
|     p11-glue#7 0x11080ee97 in provider_activate+0x117 (libcrypto.3.dylib:x86_64+0x12be97)
|     p11-glue#8 0x11080eced in ossl_provider_activate+0x42 (libcrypto.3.dylib:x86_64+0x12bced)
|     p11-glue#9 0x11080db4a in provider_conf_init+0x2a2 (libcrypto.3.dylib:x86_64+0x12ab4a)
|     p11-glue#10 0x11075ae36 in CONF_modules_load+0x37b (libcrypto.3.dylib:x86_64+0x77e36)
|     p11-glue#11 0x11075b0c9 in CONF_modules_load_file_ex+0x78 (libcrypto.3.dylib:x86_64+0x780c9)
|     p11-glue#12 0x11075b899 in ossl_config_int+0x36 (libcrypto.3.dylib:x86_64+0x78899)
|     p11-glue#13 0x110806ac9 in ossl_init_config_ossl_+0xa (libcrypto.3.dylib:x86_64+0x123ac9)
|     p11-glue#14 0x7ff818b4acc4 in __pthread_once_handler+0x40 (libsystem_pthread.dylib:x86_64+0x2cc4)
|     p11-glue#15 0x7ff818b61725 in _os_once_callout+0x11 (libsystem_platform.dylib:x86_64+0x1725)
|     p11-glue#16 0x7ff818b4ac72 in pthread_once+0x49 (libsystem_pthread.dylib:x86_64+0x2c72)
|     p11-glue#17 0x11081126c in CRYPTO_THREAD_run_once+0x8 (libcrypto.3.dylib:x86_64+0x12e26c)
|     p11-glue#18 0x11080691d in OPENSSL_init_crypto+0x446 (libcrypto.3.dylib:x86_64+0x12391d)
|     p11-glue#19 0x10f6032dc in OPENSSL_init_ssl+0x79 (libssl.3.dylib:x86_64+0x102dc)
|     p11-glue#20 0x10f3c6287 in main+0x62 (openssl:x86_64+0x100024287)
|     p11-glue#21 0x115f5252d in start+0x1cd (dyld:x86_64+0x552d)
|
| 0x616000000bb0 is located 0 bytes to the right of 560-byte region [0x616000000980,0x616000000bb0)
| allocated by thread T0 here:
|     #0 0x10f8e1ed0 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4aed0)
|     p11-glue#1 0x111f1432f in CRYPTO_malloc+0x8f (libcrypto.3.dylib:x86_64+0x16332f)
|     p11-glue#2 0x11184e1a8 in get_slot_mechanisms session.c:107
|     p11-glue#3 0x11184c3ba in p11prov_init_slots session.c:226
|     p11-glue#4 0x111843fed in p11prov_module_init provider.c:1035
|     p11-glue#5 0x1118417a9 in OSSL_provider_init provider.c:1102
|     p11-glue#6 0x11080ee97 in provider_activate+0x117 (libcrypto.3.dylib:x86_64+0x12be97)
|     p11-glue#7 0x11080eced in ossl_provider_activate+0x42 (libcrypto.3.dylib:x86_64+0x12bced)
|     p11-glue#8 0x11080db4a in provider_conf_init+0x2a2 (libcrypto.3.dylib:x86_64+0x12ab4a)
|     p11-glue#9 0x11075ae36 in CONF_modules_load+0x37b (libcrypto.3.dylib:x86_64+0x77e36)
|     p11-glue#10 0x11075b0c9 in CONF_modules_load_file_ex+0x78 (libcrypto.3.dylib:x86_64+0x780c9)
|     p11-glue#11 0x11075b899 in ossl_config_int+0x36 (libcrypto.3.dylib:x86_64+0x78899)
|     p11-glue#12 0x110806ac9 in ossl_init_config_ossl_+0xa (libcrypto.3.dylib:x86_64+0x123ac9)
|     p11-glue#13 0x7ff818b4acc4 in __pthread_once_handler+0x40 (libsystem_pthread.dylib:x86_64+0x2cc4)
|     p11-glue#14 0x7ff818b61725 in _os_once_callout+0x11 (libsystem_platform.dylib:x86_64+0x1725)
|     p11-glue#15 0x7ff818b4ac72 in pthread_once+0x49 (libsystem_pthread.dylib:x86_64+0x2c72)
|     p11-glue#16 0x11081126c in CRYPTO_THREAD_run_once+0x8 (libcrypto.3.dylib:x86_64+0x12e26c)
|     p11-glue#17 0x11080691d in OPENSSL_init_crypto+0x446 (libcrypto.3.dylib:x86_64+0x12391d)
|     p11-glue#18 0x10f6032dc in OPENSSL_init_ssl+0x79 (libssl.3.dylib:x86_64+0x102dc)
|     p11-glue#19 0x10f3c6287 in main+0x62 (openssl:x86_64+0x100024287)
|     p11-glue#20 0x115f5252d in start+0x1cd (dyld:x86_64+0x552d)
|
| SUMMARY: AddressSanitizer: heap-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1f18c) in wrap_memcpy+0x16c
| Shadow bytes around the buggy address:
|   0x1c2c00000120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x1c2c00000130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| =>0x1c2c00000170: 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa
|   0x1c2c00000180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x1c2c00000190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| Shadow byte legend (one shadow byte represents 8 application bytes):
|   Addressable:           00
|   Partially addressable: 01 02 03 04 05 06 07
|   Heap left redzone:       fa
|   Freed heap region:       fd
|   Stack left redzone:      f1
|   Stack mid redzone:       f2
|   Stack right redzone:     f3
|   Stack after return:      f5
|   Stack use after scope:   f8
|   Global redzone:          f9
|   Global init order:       f6
|   Poisoned by user:        f7
|   Container overflow:      fc
|   Array cookie:            ac
|   Intra object redzone:    bb
|   ASan internal:           fe
|   Left alloca redzone:     ca
|   Right alloca redzone:    cb
| ==27174==ABORTING
| Abort trap: 6

Signed-off-by: Clemens Lang <[email protected]>
Modified-by: Daiki Ueno <[email protected]>
@ueno ueno mentioned this pull request Feb 28, 2023
Merged
ueno pushed a commit to ueno/p11-kit that referenced this pull request Feb 28, 2023
@neverpanic @ueno
rpc: Fix two off-by-one errors identified by asan
21a2b86 
Reads after the end of the array happen when removing elements as well
as if the last element was removed. Note that the new if is required
because memmove() expects a size_t as length, so we must ensure that it
can not be negative.

The full AddressSanitizer report is:

| =================================================================
| ==27174==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x616000000bb0 at pc 0x00010f8b618d bp 0x7ff7b0b5c3c0 sp 0x7ff7b0b5bb80
| READ of size 560 at 0x616000000bb0 thread T0
|     #0 0x10f8b618c in wrap_memcpy+0x16c (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1f18c)
|     p11-glue#1 0x111a9bac5 in rpc_C_GetMechanismList+0x18c (p11-kit-client.so:x86_64+0x18ac5)
|     p11-glue#2 0x111823534 in p11prov_GetMechanismList interface.gen.c:138
|     p11-glue#3 0x11184e404 in get_slot_mechanisms session.c:113
|     p11-glue#4 0x11184c3ba in p11prov_init_slots session.c:226
|     p11-glue#5 0x111843fed in p11prov_module_init provider.c:1035
|     p11-glue#6 0x1118417a9 in OSSL_provider_init provider.c:1102
|     p11-glue#7 0x11080ee97 in provider_activate+0x117 (libcrypto.3.dylib:x86_64+0x12be97)
|     p11-glue#8 0x11080eced in ossl_provider_activate+0x42 (libcrypto.3.dylib:x86_64+0x12bced)
|     p11-glue#9 0x11080db4a in provider_conf_init+0x2a2 (libcrypto.3.dylib:x86_64+0x12ab4a)
|     p11-glue#10 0x11075ae36 in CONF_modules_load+0x37b (libcrypto.3.dylib:x86_64+0x77e36)
|     p11-glue#11 0x11075b0c9 in CONF_modules_load_file_ex+0x78 (libcrypto.3.dylib:x86_64+0x780c9)
|     p11-glue#12 0x11075b899 in ossl_config_int+0x36 (libcrypto.3.dylib:x86_64+0x78899)
|     p11-glue#13 0x110806ac9 in ossl_init_config_ossl_+0xa (libcrypto.3.dylib:x86_64+0x123ac9)
|     p11-glue#14 0x7ff818b4acc4 in __pthread_once_handler+0x40 (libsystem_pthread.dylib:x86_64+0x2cc4)
|     p11-glue#15 0x7ff818b61725 in _os_once_callout+0x11 (libsystem_platform.dylib:x86_64+0x1725)
|     p11-glue#16 0x7ff818b4ac72 in pthread_once+0x49 (libsystem_pthread.dylib:x86_64+0x2c72)
|     p11-glue#17 0x11081126c in CRYPTO_THREAD_run_once+0x8 (libcrypto.3.dylib:x86_64+0x12e26c)
|     p11-glue#18 0x11080691d in OPENSSL_init_crypto+0x446 (libcrypto.3.dylib:x86_64+0x12391d)
|     p11-glue#19 0x10f6032dc in OPENSSL_init_ssl+0x79 (libssl.3.dylib:x86_64+0x102dc)
|     p11-glue#20 0x10f3c6287 in main+0x62 (openssl:x86_64+0x100024287)
|     p11-glue#21 0x115f5252d in start+0x1cd (dyld:x86_64+0x552d)
|
| 0x616000000bb0 is located 0 bytes to the right of 560-byte region [0x616000000980,0x616000000bb0)
| allocated by thread T0 here:
|     #0 0x10f8e1ed0 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4aed0)
|     p11-glue#1 0x111f1432f in CRYPTO_malloc+0x8f (libcrypto.3.dylib:x86_64+0x16332f)
|     p11-glue#2 0x11184e1a8 in get_slot_mechanisms session.c:107
|     p11-glue#3 0x11184c3ba in p11prov_init_slots session.c:226
|     p11-glue#4 0x111843fed in p11prov_module_init provider.c:1035
|     p11-glue#5 0x1118417a9 in OSSL_provider_init provider.c:1102
|     p11-glue#6 0x11080ee97 in provider_activate+0x117 (libcrypto.3.dylib:x86_64+0x12be97)
|     p11-glue#7 0x11080eced in ossl_provider_activate+0x42 (libcrypto.3.dylib:x86_64+0x12bced)
|     p11-glue#8 0x11080db4a in provider_conf_init+0x2a2 (libcrypto.3.dylib:x86_64+0x12ab4a)
|     p11-glue#9 0x11075ae36 in CONF_modules_load+0x37b (libcrypto.3.dylib:x86_64+0x77e36)
|     p11-glue#10 0x11075b0c9 in CONF_modules_load_file_ex+0x78 (libcrypto.3.dylib:x86_64+0x780c9)
|     p11-glue#11 0x11075b899 in ossl_config_int+0x36 (libcrypto.3.dylib:x86_64+0x78899)
|     p11-glue#12 0x110806ac9 in ossl_init_config_ossl_+0xa (libcrypto.3.dylib:x86_64+0x123ac9)
|     p11-glue#13 0x7ff818b4acc4 in __pthread_once_handler+0x40 (libsystem_pthread.dylib:x86_64+0x2cc4)
|     p11-glue#14 0x7ff818b61725 in _os_once_callout+0x11 (libsystem_platform.dylib:x86_64+0x1725)
|     p11-glue#15 0x7ff818b4ac72 in pthread_once+0x49 (libsystem_pthread.dylib:x86_64+0x2c72)
|     p11-glue#16 0x11081126c in CRYPTO_THREAD_run_once+0x8 (libcrypto.3.dylib:x86_64+0x12e26c)
|     p11-glue#17 0x11080691d in OPENSSL_init_crypto+0x446 (libcrypto.3.dylib:x86_64+0x12391d)
|     p11-glue#18 0x10f6032dc in OPENSSL_init_ssl+0x79 (libssl.3.dylib:x86_64+0x102dc)
|     p11-glue#19 0x10f3c6287 in main+0x62 (openssl:x86_64+0x100024287)
|     p11-glue#20 0x115f5252d in start+0x1cd (dyld:x86_64+0x552d)
|
| SUMMARY: AddressSanitizer: heap-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1f18c) in wrap_memcpy+0x16c
| Shadow bytes around the buggy address:
|   0x1c2c00000120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x1c2c00000130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| =>0x1c2c00000170: 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa
|   0x1c2c00000180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x1c2c00000190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| Shadow byte legend (one shadow byte represents 8 application bytes):
|   Addressable:           00
|   Partially addressable: 01 02 03 04 05 06 07
|   Heap left redzone:       fa
|   Freed heap region:       fd
|   Stack left redzone:      f1
|   Stack mid redzone:       f2
|   Stack right redzone:     f3
|   Stack after return:      f5
|   Stack use after scope:   f8
|   Global redzone:          f9
|   Global init order:       f6
|   Poisoned by user:        f7
|   Container overflow:      fc
|   Array cookie:            ac
|   Intra object redzone:    bb
|   ASan internal:           fe
|   Left alloca redzone:     ca
|   Right alloca redzone:    cb
| ==27174==ABORTING
| Abort trap: 6

Signed-off-by: Clemens Lang <[email protected]>
Modified-by: Daiki Ueno <[email protected]>
ueno pushed a commit that referenced this pull request Feb 28, 2023
@neverpanic @ueno
rpc: Fix two off-by-one errors identified by asan
8fd7fc5 
Reads after the end of the array happen when removing elements as well
as if the last element was removed. Note that the new if is required
because memmove() expects a size_t as length, so we must ensure that it
can not be negative.

The full AddressSanitizer report is:

| =================================================================
| ==27174==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x616000000bb0 at pc 0x00010f8b618d bp 0x7ff7b0b5c3c0 sp 0x7ff7b0b5bb80
| READ of size 560 at 0x616000000bb0 thread T0
|     #0 0x10f8b618c in wrap_memcpy+0x16c (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1f18c)
|     #1 0x111a9bac5 in rpc_C_GetMechanismList+0x18c (p11-kit-client.so:x86_64+0x18ac5)
|     #2 0x111823534 in p11prov_GetMechanismList interface.gen.c:138
|     #3 0x11184e404 in get_slot_mechanisms session.c:113
|     #4 0x11184c3ba in p11prov_init_slots session.c:226
|     #5 0x111843fed in p11prov_module_init provider.c:1035
|     #6 0x1118417a9 in OSSL_provider_init provider.c:1102
|     #7 0x11080ee97 in provider_activate+0x117 (libcrypto.3.dylib:x86_64+0x12be97)
|     #8 0x11080eced in ossl_provider_activate+0x42 (libcrypto.3.dylib:x86_64+0x12bced)
|     #9 0x11080db4a in provider_conf_init+0x2a2 (libcrypto.3.dylib:x86_64+0x12ab4a)
|     #10 0x11075ae36 in CONF_modules_load+0x37b (libcrypto.3.dylib:x86_64+0x77e36)
|     #11 0x11075b0c9 in CONF_modules_load_file_ex+0x78 (libcrypto.3.dylib:x86_64+0x780c9)
|     #12 0x11075b899 in ossl_config_int+0x36 (libcrypto.3.dylib:x86_64+0x78899)
|     #13 0x110806ac9 in ossl_init_config_ossl_+0xa (libcrypto.3.dylib:x86_64+0x123ac9)
|     #14 0x7ff818b4acc4 in __pthread_once_handler+0x40 (libsystem_pthread.dylib:x86_64+0x2cc4)
|     #15 0x7ff818b61725 in _os_once_callout+0x11 (libsystem_platform.dylib:x86_64+0x1725)
|     #16 0x7ff818b4ac72 in pthread_once+0x49 (libsystem_pthread.dylib:x86_64+0x2c72)
|     #17 0x11081126c in CRYPTO_THREAD_run_once+0x8 (libcrypto.3.dylib:x86_64+0x12e26c)
|     #18 0x11080691d in OPENSSL_init_crypto+0x446 (libcrypto.3.dylib:x86_64+0x12391d)
|     #19 0x10f6032dc in OPENSSL_init_ssl+0x79 (libssl.3.dylib:x86_64+0x102dc)
|     #20 0x10f3c6287 in main+0x62 (openssl:x86_64+0x100024287)
|     #21 0x115f5252d in start+0x1cd (dyld:x86_64+0x552d)
|
| 0x616000000bb0 is located 0 bytes to the right of 560-byte region [0x616000000980,0x616000000bb0)
| allocated by thread T0 here:
|     #0 0x10f8e1ed0 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4aed0)
|     #1 0x111f1432f in CRYPTO_malloc+0x8f (libcrypto.3.dylib:x86_64+0x16332f)
|     #2 0x11184e1a8 in get_slot_mechanisms session.c:107
|     #3 0x11184c3ba in p11prov_init_slots session.c:226
|     #4 0x111843fed in p11prov_module_init provider.c:1035
|     #5 0x1118417a9 in OSSL_provider_init provider.c:1102
|     #6 0x11080ee97 in provider_activate+0x117 (libcrypto.3.dylib:x86_64+0x12be97)
|     #7 0x11080eced in ossl_provider_activate+0x42 (libcrypto.3.dylib:x86_64+0x12bced)
|     #8 0x11080db4a in provider_conf_init+0x2a2 (libcrypto.3.dylib:x86_64+0x12ab4a)
|     #9 0x11075ae36 in CONF_modules_load+0x37b (libcrypto.3.dylib:x86_64+0x77e36)
|     #10 0x11075b0c9 in CONF_modules_load_file_ex+0x78 (libcrypto.3.dylib:x86_64+0x780c9)
|     #11 0x11075b899 in ossl_config_int+0x36 (libcrypto.3.dylib:x86_64+0x78899)
|     #12 0x110806ac9 in ossl_init_config_ossl_+0xa (libcrypto.3.dylib:x86_64+0x123ac9)
|     #13 0x7ff818b4acc4 in __pthread_once_handler+0x40 (libsystem_pthread.dylib:x86_64+0x2cc4)
|     #14 0x7ff818b61725 in _os_once_callout+0x11 (libsystem_platform.dylib:x86_64+0x1725)
|     #15 0x7ff818b4ac72 in pthread_once+0x49 (libsystem_pthread.dylib:x86_64+0x2c72)
|     #16 0x11081126c in CRYPTO_THREAD_run_once+0x8 (libcrypto.3.dylib:x86_64+0x12e26c)
|     #17 0x11080691d in OPENSSL_init_crypto+0x446 (libcrypto.3.dylib:x86_64+0x12391d)
|     #18 0x10f6032dc in OPENSSL_init_ssl+0x79 (libssl.3.dylib:x86_64+0x102dc)
|     #19 0x10f3c6287 in main+0x62 (openssl:x86_64+0x100024287)
|     #20 0x115f5252d in start+0x1cd (dyld:x86_64+0x552d)
|
| SUMMARY: AddressSanitizer: heap-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1f18c) in wrap_memcpy+0x16c
| Shadow bytes around the buggy address:
|   0x1c2c00000120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x1c2c00000130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c00000160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| =>0x1c2c00000170: 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa
|   0x1c2c00000180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x1c2c00000190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x1c2c000001c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| Shadow byte legend (one shadow byte represents 8 application bytes):
|   Addressable:           00
|   Partially addressable: 01 02 03 04 05 06 07
|   Heap left redzone:       fa
|   Freed heap region:       fd
|   Stack left redzone:      f1
|   Stack mid redzone:       f2
|   Stack right redzone:     f3
|   Stack after return:      f5
|   Stack use after scope:   f8
|   Global redzone:          f9
|   Global init order:       f6
|   Poisoned by user:        f7
|   Container overflow:      fc
|   Array cookie:            ac
|   Intra object redzone:    bb
|   ASan internal:           fe
|   Left alloca redzone:     ca
|   Right alloca redzone:    cb
| ==27174==ABORTING
| Abort trap: 6

Signed-off-by: Clemens Lang <[email protected]>
Modified-by: Daiki Ueno <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmav nmav nmav left review comments

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
future
Development

Successfully merging this pull request may close these issues.
2 participants
@ueno @nmav