Skip to content
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

Memory leak bugs when a new reference is only passed to a non-stealing API (static analyzer reports) #6323

Closed
Snape3058 opened this issue May 23, 2022 · 5 comments · Fixed by #7003
Labels

Comments

@Snape3058
Copy link

  • API PyDict_SetItemString does not steal a reference from the third argument.
  • API PyDict_SetItem does not steal a reference from the last argument.
  • API PyList_Append does not steal a reference from the second argument.
  • API PyDict_SetItem does not steal a reference from the third argument if the return value is not zero.

If a new reference is passed to the function without decreasing its refcnt then, it will lead to a memory leak.


Pattern 1: APIs returning a new reference are called directly as the third argument.

  • Internal Report ID: 1b7403
    PyDict_SetItemString(d, "new_count", PyLong_FromLong(arena->stats_new_count));
  • Internal Report ID: 6fa78e
    d, "allocated_blocks", PyLong_FromLong(arena->stats_allocated_blocks));
  • Internal Report ID: b43c87
    d, "reused_blocks", PyLong_FromLong(arena->stats_reused_blocks));
  • Internal Report ID: 1640ab
    d, "reallocated_blocks", PyLong_FromLong(arena->stats_reallocated_blocks));
  • Internal Report ID: 78d881
    PyDict_SetItemString(d, "freed_blocks", PyLong_FromLong(arena->stats_freed_blocks));
  • Internal Report ID: 6acded
    PyDict_SetItemString(d, "blocks_cached", PyLong_FromLong(arena->blocks_cached));
  • Internal Report ID: 15cbc4
    d, "jpeglib_version", PyUnicode_FromString(ImagingJpegVersion()));
  • Internal Report ID: a78d30
    d, "jp2klib_version", PyUnicode_FromString(ImagingJpeg2KVersion()));
  • Internal Report ID: 446850
    d, "libjpeg_turbo_version", PyUnicode_FromString(tostr(LIBJPEG_TURBO_VERSION)));
  • Internal Report ID: 3c8041
    d, "imagequant_version", PyUnicode_FromString(ImagingImageQuantVersion()));
  • Internal Report ID: 1abe89
    d, "zlib_version", PyUnicode_FromString(ImagingZipVersion()));
  • Internal Report ID: b9e74a
    d, "libtiff_version", PyUnicode_FromString(ImagingTiffVersion()));
  • Internal Report ID: be23b5
    PyDict_SetItemString(d, "PILLOW_VERSION", PyUnicode_FromString(version));
  • Internal Report ID: 611fdf
    list_axis, "minimum", PyLong_FromLong(axis.minimum / 65536));
  • Internal Report ID: 7c0d23
    PyDict_SetItemString(list_axis, "default", PyLong_FromLong(axis.def / 65536));
  • Internal Report ID: b84747
    list_axis, "maximum", PyLong_FromLong(axis.maximum / 65536));
  • Internal Report ID: 3c802a
    PyDict_SetItemString(d, "__version", PyUnicode_FromString("0.1"));
  • Internal Report ID: 71bbbc
    d, "webpdecoder_version", PyUnicode_FromString(WebPDecoderVersion_str()));
  • Internal Report ID: 213466
    m, "HAVE_TRANSPARENCY", PyBool_FromLong(!WebPDecoderBuggyAlpha()));

Pattern 2: Intermediate variables are used to forward the argument.

  • Internal Report ID: e4a37b

New reference is returned here:

v = PyUnicode_FromFormat("%d.%d.%d", vn / 1000, (vn / 10) % 100, vn % 10);

PyObject is passed to non-stealing API here:
PyDict_SetItemString(d, "littlecms_version", v);


  • Internal Report ID: dc5fd5

New reference is returned here:

v = PyUnicode_FromFormat("%d.%d", vn / 1000, (vn / 10) % 100);

PyObject is passed to non-stealing API here:
PyDict_SetItemString(d, "littlecms_version", v);


  • Internal Report ID: 1e988b

New reference is returned here:

id = PyLong_FromLong((long)intent);

PyObject is passed to non-stealing API here:
PyDict_SetItem(result, id, entry);


  • Internal Report ID: 0ef1f7

New reference is returned here:

entry = Py_BuildValue(

PyObject is passed to non-stealing API here:
PyDict_SetItem(result, id, entry);


  • Internal Report ID: fce490

New reference is returned here:

axis_name = Py_BuildValue("y#", name.string, name.string_len);

PyObject is passed to non-stealing API here:
PyDict_SetItemString(list_axis, "name", axis_name);


  • Internal Report ID: d17f14

New reference is returned here:

v = PyUnicode_FromFormat("%d.%d.%d", major, minor, patch);

PyObject is passed to non-stealing API here:
PyDict_SetItemString(d, "freetype2_version", v);


  • Internal Report ID: 21a68f

New reference is returned here:

v = PyUnicode_FromString(raqm_version_string());

PyObject is passed to non-stealing API here:
PyDict_SetItemString(d, "raqm_version", v);


  • Internal Report ID: e80f52

New reference is returned here:

v = PyBool_FromLong(have_raqm);

PyObject is passed to non-stealing API here:
PyDict_SetItemString(d, "HAVE_RAQM", v);

PyDict_SetItemString(d, "HAVE_FRIBIDI", v);

PyDict_SetItemString(d, "HAVE_HARFBUZZ", v);


  • Internal Report ID: 29a870

New reference is returned here:

PyObject *coordObj = Py_BuildValue("(nn)", col_idx, row_idx);

PyObject is passed to non-stealing API here:
PyList_Append(ret, coordObj);


  • Internal Report ID: fddf55

New reference is returned here:

PyObject *coordObj = Py_BuildValue("(nn)", col_idx, row_idx);

PyObject is passed to non-stealing API here:
PyList_Append(ret, coordObj);

@radarhere
Copy link
Member

Would you be able to collect any more of these into a single issue?

@Snape3058
Copy link
Author

These are all the cases reported by my static analyzer. To find all of them, I suggest filtering and verifying the entire project for these four APIs.

@radarhere
Copy link
Member

Would you mind providing some detail on how you ran the static analyzer? What tool did you use specifically?

@Snape3058
Copy link
Author

It is an experimental analyzer of my unpublished research, which is developed on the top of the clang static analyzer.

@radarhere radarhere changed the title BUG: Memory leak bugs when a new reference is only passed to a non-stealing API (static analyzer reports) Memory leak bugs when a new reference is only passed to a non-stealing API (static analyzer reports) Aug 6, 2022
@radarhere
Copy link
Member

I've created PR #7003 to resolve this.

API PyDict_SetItem does not steal a reference from the third argument if the return value is not zero.

To clarify, there's a copy paste error here. The text says "PyDict_SetItem", but the link is to "PyModule_AddObject". This statement applies to "PyModule_AddObject".

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

Successfully merging a pull request may close this issue.

2 participants