search

iustin / pyxattr

A python module for accessing filesystem Extended Attributes
https://pyxattr.k1024.org/
GNU Lesser General Public License v2.1
31 stars 14 forks source link

BUG: Memory leak bugs due to returned new reference is not decreased on failure (static analyzer reports) #35

Closed Snape3058 closed 2 years ago

Snape3058 commented 2 years ago

Returning a new reference here: https://github.com/iustin/pyxattr/blob/c3466e74a2d72ede0d121aabdf687fa8d348bfc6/xattr.c#L632 Variable my_tuple goes out of scope without decreasing the refcnt. https://github.com/iustin/pyxattr/blob/c3466e74a2d72ede0d121aabdf687fa8d348bfc6/xattr.c#L639 Internal Report ID: 19f80b

Returning a new reference here: https://github.com/iustin/pyxattr/blob/c3466e74a2d72ede0d121aabdf687fa8d348bfc6/xattr.c#L1185 Error handling code after goto target does not decrease the refcnt. (also other gotos in this function) https://github.com/iustin/pyxattr/blob/c3466e74a2d72ede0d121aabdf687fa8d348bfc6/xattr.c#L1201 Internal Report ID: 1eed62

iustin commented 2 years ago

Thanks for the report, much appreciated! I'll take a look in some while, right now away from proper machine to test.

Just for my curiosity, which analyser did you use? I'd like to integrate it in GitHub hooks or at least release process, if it's freely available.

Snape3058 commented 2 years ago

It is an experimental analyzer developed on the top of Clang Static Analyzer. When the tool is publicly available, I will reply to this issue to tell you. Currently, I am busy writing the paper for this research.

iustin commented 2 years ago

Sounds good, thank you - and thanks for finding the bugs!

iustin commented 2 years ago

I've committed a fix for the first issue, which was very clear. For the second batch of issues, in the module initalisation, I'm not entirely sure what's the correct way of handling initialisation error - is deconstructing (via ref count decrease) the module the right thing? It seems so, but testing behaviour here is hard.

iustin commented 2 years ago

Ah, actually the tutorial shows exactly this. Then it's a clear and simple fix.