php / php-src

The PHP Interpreter
https://www.php.net
Other
37.94k stars 7.73k forks source link

shmop_open should throw ValueError for ids overflowing sizeof int #9945

Open TysonAndre opened 1 year ago

TysonAndre commented 1 year ago

Description

The following code:

<?php // based on ext/shmop/tests/shmop_open_private.phpt
$write = 'test';

$shm1 = shmop_open(0x2_0000_0001, 'c', 0777, 1024);
shmop_write($shm1, $write, 0);

$shm2 = shmop_open(0x1_0000_0001, 'c', 0777, 1024);
$read = shmop_read($shm2, 0, 4);

var_dump(is_string($read) && $read !== $write);

Resulted in this output:

bool(false)

But I expected this output instead:

bool(true)

(seen on 64-bit linux with sizeof(int) == 4, sizeof(zend_long) == 8)

Not really something users are likely to do deliberately

This also means that 0x1_0000_0000 is an alias for IPC_PRIVATE, which always creates a new memory segment.

Noticed while looking into #9944

PHP Version

Any

Operating System

No response

cmb69 commented 1 year ago

Yeah, that behavior is not correct; however, key_t is underspecified by POSIX; it merely mandates that key_t is an arithmetic type, so we probably can't assume it is int everywhere.

TysonAndre commented 1 year ago

Yeah, that behavior is not correct; however, key_t is underspecified by POSIX; it merely mandates that key_t is an arithmetic type, so we probably can't assume it is int everywhere.

key != (zend_long)(key_t)key should work as a runtime check.

cmb69 commented 1 year ago

key != (zend_long)(key_t)key should work as a runtime check.

If key_t is signed and its rank is less than that of zend_long, wouldn't that possibly be undefined behavior.

Anyhow, shm_attach() has exactly the same issue than shmop_open().

cmb69 commented 1 year ago

it merely mandates that key_t is an arithmetic type

This is ugly, since arithmetic types even include float etc. However, we can hopefully assume that it is actually an integer type, and as such could determine the details during configuration: its size (sizeof(key_t), and its signedness.