Skip to content

Fix GH-12621: browscap segmentation fault when configured in the vhost #12634

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

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Nov 8, 2023

The temporary HashTable has a destructor that releases the string held by the entry's value. However, browscap_intern_str(_ci) only incremented the refcount for the reference created by the return value. As the HashTable is only used during parsing, we don't need to manage the reference count of the value anyway, so get rid of the destructor.

Although the original bug report is for Apache, it is easily triggerable in fpm too, so use that for the test.

This is triggerable in two cases:

  • When using php_admin_value to set the ini at the activation stage
  • When running out of space for the opcache-interned strings

Targeting 8.2 because 8.1 is no longer open for bugfixes, right?

…host

The temporary HashTable has a destructor that releases the string held
by the entry's value. However, browscap_intern_str(_ci) only incremented
the refcount for the reference created by the return value. As the
HashTable is only used during parsing, we don't need to manage the
reference count of the value anyway, so get rid of the destructor.

This is triggerable in two cases:
 - When using php_admin_value to set the ini at the activation stage
 - When running out of space for the opcache-interned strings
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Looks sensible to me.

Also yes AFAIU it should now go into 8.2

@nielsdos nielsdos closed this in 7353c7c Nov 11, 2023
ramsey pushed a commit that referenced this pull request Nov 23, 2023
The temporary HashTable has a destructor that releases the string held
by the entry's value. However, browscap_intern_str(_ci) only incremented
the refcount for the reference created by the return value. As the
HashTable is only used during parsing, we don't need to manage the
reference count of the value anyway, so get rid of the destructor.

This is triggerable in two cases:
 - When using php_admin_value to set the ini at the activation stage
 - When running out of space for the opcache-interned strings

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

Successfully merging this pull request may close these issues.

browscap segmentation fault when configured in the vhost
2 participants