[issue27945] Various segfaults with dict

Change by Ned Deily : -- Removed message: https://bugs.python.org/msg342104 ___ Python tracker ___ ___ Python-bugs-list mailing

[issue27945] Various segfaults with dict

Ned Deily added the comment: New changeset f7344798e57da6b9c4ed9372e8eaecde80989c86 by larryhastings (Serhiy Storchaka) in branch '3.4': [3.4] [3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (GH-1678) (#2248)

[issue27945] Various segfaults with dict

Changes by Ned Deily : -- priority: release blocker -> resolution: -> fixed stage: commit review -> resolved status: open -> closed ___ Python tracker

[issue27945] Various segfaults with dict

Ned Deily added the comment: New changeset 8fbdab50fc8f2b71f19b54f3a0208cfbf2be7713 by Ned Deily (Serhiy Storchaka) in branch '3.3': [3.3] [3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (GH-1678) (#2396)

[issue27945] Various segfaults with dict

Larry Hastings added the comment: New changeset f7344798e57da6b9c4ed9372e8eaecde80989c86 by larryhastings (Serhiy Storchaka) in branch '3.4': [3.4] [3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (GH-1678) (#2248)

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: For the context see issue30484. If the segfault is caused by garbage collection this perhaps is not an expluatable vulnerability, but a severe bug that can affect any multithread application and doesn't have a workaround. --

[issue27945] Various segfaults with dict

Ned Deily added the comment: Since Serhiy created backport PRs for 3.4 and 3.3, I'm reopening the issue and marking it as Release Blocker (for those releases) so we don't lose track of them and agree they meet the criteria for security-fix-only releases. -- nosy: +benjamin.peterson,

[issue27945] Various segfaults with dict

Changes by Serhiy Storchaka : -- pull_requests: +2442 ___ Python tracker ___ ___

[issue27945] Various segfaults with dict

Changes by Serhiy Storchaka : -- pull_requests: +2296 ___ Python tracker ___ ___

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: Thank you for your patches Duane and Tim. Thank you for your very detailed report that allow writing these patches tehybel. I excluded changes in dict.fromkeys() since they look unnecessary for this issue after fixing insertdict(). There are other reasons

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: New changeset e6a0b5982973e64b9fa28e5e3e54eb8c47882780 by Serhiy Storchaka in branch '2.7': [2.7] bpo-27945: Fixed various segfaults with dict. (GH-1657) (#1681) https://github.com/python/cpython/commit/e6a0b5982973e64b9fa28e5e3e54eb8c47882780 --

[issue27945] Various segfaults with dict

Changes by Serhiy Storchaka : -- pull_requests: +1776 ___ Python tracker ___ ___

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: New changeset 564398af6ccb34d0db8b6e2537830eca285689e5 by Serhiy Storchaka in branch '3.6': [3.6] bpo-27945: Fixed various segfaults with dict. (GH-1657) (#1677) https://github.com/python/cpython/commit/564398af6ccb34d0db8b6e2537830eca285689e5 --

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: New changeset 2f7f533cf6fb57fcedcbc7bd454ac59fbaf2c655 by Serhiy Storchaka in branch '3.5': [3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (#1678) https://github.com/python/cpython/commit/2f7f533cf6fb57fcedcbc7bd454ac59fbaf2c655 --

[issue27945] Various segfaults with dict

Changes by Serhiy Storchaka : -- pull_requests: +1773 ___ Python tracker ___ ___

[issue27945] Various segfaults with dict

Changes by Serhiy Storchaka : -- pull_requests: +1772 ___ Python tracker ___ ___

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: New changeset 753bca3934a7618a4fa96e107ad1c5c18633a683 by Serhiy Storchaka in branch 'master': bpo-27945: Fixed various segfaults with dict. (#1657) https://github.com/python/cpython/commit/753bca3934a7618a4fa96e107ad1c5c18633a683 --

[issue27945] Various segfaults with dict

Changes by Serhiy Storchaka : -- pull_requests: +1751 ___ Python tracker ___ ___

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: This issue is severe, but I don't consider it as release critical for 3.6.0. The patch fixes segfaults, but it can add unneeded overhead, and the dict performance is critical for Python core. The segfaults are not new. I'm trying to minimize the overhead of

[issue27945] Various segfaults with dict

Ned Deily added the comment: Where do we stand on this issue? At the moment, 3.6.0 is going to be released without these fixes. -- ___ Python tracker

[issue27945] Various segfaults with dict

INADA Naoki added the comment: LGTM. Performance on Azure VM (AMD Opteron(tm) Processor 4171 HE): $ ~/local/py36/bin/patched -m perf compare_to master.json patched.json -G Slower (10): - spectral_norm: 915 ms +- 17 ms -> 967 ms +- 25 ms: 1.06x slower - nbody: 774 ms +- 28 ms -> 805 ms +- 22

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: The change to dict_equal() LGTM. It doesn't add an overhead. For dictiter_iternextitem() I propose other change. It doesn't add an overhead. There are bugs in the patch for _PyDict_FromKeys(). The change to dictitems_contains() adds an overhead, but it is

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: Here is a consolidated patch for review. -- Added file: http://bugs.python.org/file45549/27945-dict-segv-py36.patch ___ Python tracker

[issue27945] Various segfaults with dict

INADA Naoki added the comment: I modified the patch to avoid incref when pair is not list, because tuple is common for such case. But I can't get back original performance. (python 2.7 is modified version of patch) inada-n@test1:~/work/bench$ ~/local/py27/bin/master -m perf timeit --rigorous

[issue27945] Various segfaults with dict

INADA Naoki added the comment: I'm sorry, dict.fromkeys() didn't use PyDict_MergeFromSeq2(). This may be microbench for worst case: $ ~/local/py35/bin/master -m perf timeit --rigorous --python ~/local/py35/bin/patched --compare-to ~/local/py35/bin/master -s 'L = [(i,i) for i in

[issue27945] Various segfaults with dict

INADA Naoki added the comment: Only patch which affects to hot loop is: --- a/Objects/dictobject.c Tue Nov 15 21:21:35 2016 -0500 +++ b/Objects/dictobject.c Wed Nov 16 11:40:51 2016 + @@ -1550,11 +1550,18 @@ PyDict_MergeFromSeq2(PyObject *d, PyObje /* Update/merge with

[issue27945] Various segfaults with dict

INADA Naoki added the comment: I run performance 0.5.0 on Python 3.5. Since it took long time even without -b all option, I haven't run it for Python 2.7 yet. On Python 3.5: $ ./venv/cpython3.5-846d5b1f0b61/bin/python -m perf compare_to py35-master.json py35-patched.json -G Slower (14): -

[issue27945] Various segfaults with dict

INADA Naoki added the comment: OK, I'll run benchmark in this week. But three patches seems don't affects to performance critical APIs. -- ___ Python tracker

[issue27945] Various segfaults with dict

Serhiy Storchaka added the comment: I worry about performance. Additional increment/decrement of reference counter can add significant overhead in critical places of Python interpreter. Before committing any patches we should measure the performance lost. --

[issue27945] Various segfaults with dict

INADA Naoki added the comment: 0001-Issue-27945-fix-dictiter_iternextitem-use-after-free.patch LGTM and OK too. But 0001-Issue-27945-Fixed-segfaults-in-dict.fromkeys-when-it.patch cause conflict. I want to commit first three patches. For another reviewer, here is the patch merging three

[issue27945] Various segfaults with dict

INADA Naoki added the comment: 0001-Issue-27945-fix-dictitems_contains-use-after-free.patch LGTM. This patch can be applied to 2.7 and 3.5, without conflict against previous patch. It passes `make quicktest`. -- ___ Python tracker

[issue27945] Various segfaults with dict

INADA Naoki added the comment: 0001-Issue-27945-fix-PyDict_MergeFromSeq2-use-after-free.patch: LGTM. I've checked it can be applied to 2.7 and 3.5 branch and passes `make quicktest`. -- ___ Python tracker

[issue27945] Various segfaults with dict

Changes by Serhiy Storchaka : -- nosy: +inada.naoki versions: +Python 2.7, Python 3.7 ___ Python tracker ___

[issue27945] Various segfaults with dict

Duane Griffin added the comment: Note that I think most or all of these issues apply to 2.7 and while I didn't do a proper check I think the fixes also apply. -- ___ Python tracker

[issue27945] Various segfaults with dict

Tim Mitchell added the comment: Here is my patch for parts 3 and 4. Core issue for part 4 appears to be dk_lookup calling arbitrary python code may free the key. dk_lookup is also used in _PyDict_LoadGlobal not sure if this bug can occur here. -- nosy: +Tim Mitchell Added file:

[issue27945] Various segfaults with dict

Changes by Serhiy Storchaka : -- assignee: -> serhiy.storchaka stage: needs patch -> patch review ___ Python tracker ___

[issue27945] Various segfaults with dict

Changes by Duane Griffin : Removed file: http://bugs.python.org/file44573/0001-Issue-27945-fix-dictiter_iternextitem-use-after-free.patch ___ Python tracker

[issue27945] Various segfaults with dict

Duane Griffin added the comment: Ah, my first fix (for the fifth issue) was incomplete. Please see attached patch which I think correctly fixes the problem. -- Added file: http://bugs.python.org/file44585/0001-Issue-27945-fix-dictiter_iternextitem-use-after-free.patch

[issue27945] Various segfaults with dict

Duane Griffin added the comment: Fix for the second issue: with this fix there is no segfault or valgrind issue reported on during execution or on exit. -- Added file: http://bugs.python.org/file44582/0001-Issue-27945-fix-dictitems_contains-use-after-free.patch

[issue27945] Various segfaults with dict

Duane Griffin added the comment: Fix for the first issue: with this fix there is no segfault or valgrind issue reported on during execution or on exit. -- Added file: http://bugs.python.org/file44579/0001-Issue-27945-fix-PyDict_MergeFromSeq2-use-after-free.patch

[issue27945] Various segfaults with dict

Duane Griffin added the comment: Apologies: compiling python with --with-pydebug all of these issues are reproducible on head after all. Furthermore while my patch fixes the reported crash it still crashes on exit: Program received signal SIGSEGV, Segmentation fault. 0x00437193 in

[issue27945] Various segfaults with dict

Duane Griffin added the comment: I cannot reproduce the segfaults for the first four issues however valgrind still reports problems for all but the second. The fifth (last) one still segfaults. I have a patch for the fifth issue. The other remaining issues are all reporting the same invalid

[issue27945] Various segfaults with dict

Emanuel Barry added the comment: Ping. The built-in dict was considerably changed in #27350; do any of these issues still persist? -- ___ Python tracker

[issue27945] Various segfaults with dict

Changes by Christian Heimes : -- nosy: +christian.heimes ___ Python tracker ___ ___

[issue27945] Various segfaults with dict

Changes by Emanuel Barry : -- nosy: +ebarry, larry, ned.deily, rhettinger, serhiy.storchaka priority: normal -> critical stage: -> needs patch title: five dictobject issues -> Various segfaults with dict type: -> crash ___