This code was refactored and the unlock was forgotten.
The following assertion is triggered in debug mode:
zend_shared_alloc_lock: Assertion `!(accel_globals.locked)' failed.
And in release mode this likely deadlocks.
Fix this by re-adding the unlock.
memsize is a signed long, therefore the check against the
(*un*signed long maximum) / 1024² will allow too large values. This check worked
correctly in d4b3f89c53 where it checked against the maximum signed
value, but was broken in 003346c450. Fix it by changing ZEND_ULONG_MAX
to ZEND_LONG_MAX.
Generators that suspended a fiber should not be dtor because they will be
executed during the fiber dtor.
Fiber dtor throws an exception in the fiber's context in order to unwind and
execute finally blocks, which will also properly dtor the generator.
Fixes GH-9916
With a single byte we can choose offsets between 0x00 and 0xff, thus 0x100
different offsets. We only need to use the slow path for sources of more than
0x100 bytes.
The previous version was correct with regard to the output expectations, it was
just slower than necessary. Better fix this now while we still can before being
bound by our BC guarantees with regard to emitted sequences.
This also adds a test to verify the behavior: For powers of two we never reject
any values during rejection sampling, we just need to mask off the unneeded
bits. Thus we can specifically verify that the number of calls to the engine
match the expected amount. We also verify that all the possible values are
emitted to make sure the masking does not remove any required bits. For inputs
longer than 0x100 bytes we need trust the `range()` implementation to be
unbiased, but still verify the number of engine calls and perform a basic
output check.
In a GitHub thread, Michael Voříšek and Kamil Tekiela mentioned that
the PCRE2 function `pcre_match` can be used to validate UTF-8, and that
historically it was more efficient than mbstring's `mb_check_encoding`.
`mb_check_encoding` is now much faster on hosts with SSE2, and much
faster again on hosts with AVX2. However, while all x86-64 CPUs support
at least SSE2, not all PHP users run their code on x86-64 hardware.
For example, some use recent Macs with ARM CPUs.
Therefore, borrow PCRE2's UTF-8 validation function as a fallback for
hosts with no SSE2/AVX2 support. On long UTF-8 strings, this code is
50% faster than mbstring's existing fallback code.
From some local benchmarks which I ran, the AVX2-based version is about
2.8x faster than the SSE2-based version on long (~10,000 byte) strings,
1.6x faster on medium (~100 byte) strings, and just about the same
on very short strings.
I followed the example of the code in the 'standard' module, using
preprocessor directives so that the code can be compiled in any of
4 ways:
1) With no AVX2 support at all (for example, when PHP is compiled for
CPU architectures other than AMD64)
2) For CPUs with AVX2 only (for example, when PHP is built with
CCFLAGS='-march=native' on a host which implements AVX2)
3) With runtime detection of AVX2 performed by the dynamic linker;
this requires a dynamic linker which supports the STT_GNU_IFUNC
symbol type extension to the ELF binary standard. This is true of
glibc's dynamic linker, as of late 2009.
4) With runtime detection of AVX2 performed by the module init function.
The detection is done by checking the output of CPUID and then a
function pointer is set accordingly. In this case, all calls to the
UTF-8 validation routine are indirect calls through that
function pointer.
This code is a few percent faster for short UTF-8 strings. For long
(~10,000 byte) strings, it is also consistently faster on my local
microbenchmarks, but by less than 1%.
* random: Convert the urandom loop into a while() loop
This allows us to more easily reduce the scope of `n` in a future commit and
now matches the getrandom(2) loop.
* random: Move the errno reset immediately above the getrandom(2) call
* random: Reduce the scope of `n` in the CSPRNG
* random: Declare `n` outside of preprocessor branch
This debug code is part of arginfo validation. This validation will
never trigger properly because the OR operation makes the first if
always true. Fix it by changing to an AND.
Closes GH-10417
Signed-off-by: George Peter Banyard <girgias@php.net>
The condition `code == 0x0450 || code == 0x045D` is always false because
of an incorrect range check on code.
According to the BMP coverage in the encoding spec for ISO-8859-5
(https://encoding.spec.whatwg.org/iso-8859-5-bmp.html) the range of
valid characters is 0x0401 - 0x045F (except for 0x040D, 0x0450, 0x045D).
The current check has an upper bound of 0x044F instead of 0x045F.
Fix this by changing the upper bound.
Closes GH-10399
Signed-off-by: George Peter Banyard <girgias@php.net>
The current check always evaluated to false because if `!page_size`
is true, then `page_size & (page_size - 1)` equals `0 & (0 - 1)` which
is always 0. The if condition is meant to check if page_size is zero or
not a power of two, thus we must change the AND to an OR to fix this
issue.
Closes GH-10427
Signed-off-by: George Peter Banyard <girgias@php.net>
The "nothing to do" case would never be hit because the switch block
would execute if the opcode is ZEND_ASSIGN_STATIC_PROP_OP,
not ZEND_ASSIGN_STATIC_PROP. This meant that we were falling through to
the else block. Fix this by correcting the check condition.
When a uint8_t is bitshifted to the left, it is actually promoted to an
int. For the current code this has the effect of a wrong sign-extension,
and the result will also wrongly become zero when insert_pos >= 32.
Fix this by adding an explicit cast.
Furthermore, the partial prefix byte mask was computed incorrectly: the
byte is already shifted so the mask should not account for the shift.
The CSPRNG failing should be rare nowadays, but it *might* happen and without
this patch it's hard for the user to find out why the salt generation failed:
The error message is not actionable.
This patch will automatically set the CSPRNG exception to the `$previous`
exception of the ValueError that is thrown, allowing the developer to determine
the cause of the salt generation failure.
Before:
Fatal error: Uncaught ValueError: Unable to generate salt in php-src/test3.php:3
Stack trace:
#0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y')
#1 {main}
thrown in php-src/test3.php on line 3
After:
Fatal error: Uncaught Random\RandomException: Cannot open /dev/urandom: No such file or directory in php-src/test3.php:3
Stack trace:
#0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y')
#1 {main}
Next ValueError: Unable to generate salt in php-src/test3.php:3
Stack trace:
#0 php-src/test3.php(3): password_hash(Object(SensitiveParameterValue), '2y')
#1 {main}
thrown in php-src/test3.php on line 3
The only way the previous `if (read_bytes < size)` branch could be taken is
when the loop was exited by the `break;` statement. We can just merge this into
the loop to make the code more obvious.
This effectively reverts #8984.
As discussed in #10327 which will enable the use of the getrandom(2) syscall on
NetBSD instead of relying on the userland arc4random_buf(), the CSPRNG should
prioritize security over speed [1] and history has shown that userland
implementations unavoidably fall short on the security side. In fact the glibc
implementation is a thin wrapper around the syscall due to security concerns
and thus does not provide any benefit over just calling getrandom(2) ourselves.
Even without any performance optimizations the CSPRNG should be plenty fast for
the vast majority of applications, because they often only need a few bytes of
randomness to generate a session ID. If speed is desired, the OO API offers
faster, but non-cryptographically secure engines.
