Skip to content

shmop_open should throw ValueError for ids overflowing sizeof int #9945

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

Open
TysonAndre opened this issue Nov 13, 2022 · 4 comments
Open

shmop_open should throw ValueError for ids overflowing sizeof int #9945

TysonAndre opened this issue Nov 13, 2022 · 4 comments

Comments

@TysonAndre
Copy link
Contributor

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
Copy link
Member

cmb69 commented Nov 14, 2022

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
Copy link
Contributor Author

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
Copy link
Member

cmb69 commented Nov 15, 2022

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
Copy link
Member

cmb69 commented Nov 15, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants