ldx / python-iptables

Python bindings for iptables
730 stars 182 forks source link

Double free on _Buffer destruction #317

Closed nirizr closed 3 years ago

nirizr commented 3 years ago

Hi,

I hope this is the right format for asking, please refer if not.

I'm having an occasional double free segmentation fault when ip4tc._Buffer.__del__ is called (presumably by the garbage collector), and I'm not even sure if it's a bug caused by me or by ip4tc (which I understand is part of python-iptables), using version python-iptables-1.0.0.

The last traceback I got is this, however the tracebacks change and nearly always end at the __del__ call:

double free or corruption (fasttop)
Fatal Python error: Aborted

Thread 0x00007f5f92ffd700 (most recent call first):
  File "/usr/lib/python3.8/threading.py", line 306 in wait
  File "/usr/lib/python3.8/queue.py", line 179 in get
  File "/usr/local/lib/python3.8/dist-packages/watchdog/observers/api.py", line 360 in dispatch_events
  File "/usr/local/lib/python3.8/dist-packages/watchdog/observers/api.py", line 199 in run
  File "/project/src/Packages/logging_utils/guid_logger_utils/guid_logging_helpers.py", line 41 in patched_run
  File "/usr/lib/python3.8/threading.py", line 932 in _bootstrap_inner
  File "/usr/lib/python3.8/threading.py", line 890 in _bootstrap

Current thread 0x00007f604a05b740 (most recent call first):
  File "/usr/local/lib/python3.8/dist-packages/iptc/ip4tc.py", line 501 in __del__
  File "/usr/lib/python3.8/copy.py", line 262 in _reconstruct
  File "/usr/lib/python3.8/copy.py", line 172 in deepcopy
  File "/usr/lib/python3.8/copy.py", line 229 in _deepcopy_dict
  File "/usr/lib/python3.8/copy.py", line 146 in deepcopy
  File "/usr/lib/python3.8/copy.py", line 269 in _reconstruct
  File "/usr/lib/python3.8/copy.py", line 172 in deepcopy
  File "/usr/lib/python3.8/copy.py", line 204 in _deepcopy_list
  File "/usr/lib/python3.8/copy.py", line 146 in deepcopy
  <snipped python files of my own, calling deepcopy>

The last iptables related operation was a deletion of a rule with the following parameters:

table=nat chain=POSTROUTING jump=MASQUERADE flt_src=192.168.13.0/24 persistence=False

I have some code that sets up iptables rules, uses them for a while (5-15 minutes) and than deletes the rules. The crash does not happen when I execute the code once, but frequently happens when I execute the above mentioned logic several times in a loop (usually after 20-70 iterations, I'll get a similar crash).

First and foremost, I will appreciate any assistance with hunting down the root cause (what to look for? what to verify/check? What could be the cause of the underlying issue?)

Additionally, I'd suspect the _Buffer class could benefit copy-protection (i.e. implementing __copy__ and __deepcopy__). If that is desirable, please let me know and I'll issue a PR.

Thanks!

ldx commented 3 years ago

The single most useful thing is a minimal example that reproduces the problem. Any chance you can reduce your code to a short function or two that does that?

Another thing that might be a problem is if you try to share iptc objects between threads.

nirizr commented 3 years ago

Thanks for your quick reply!

Unfortunately, the code that uses iptc is quite large in my codebase. Has several wrappers, etc... so pinpointing the buggy area will be a bit difficult, especially since I didn't write it.

I'm trying to do that now and I'll start by looking objects shared between threads. Any more similar pitfalls I should be looking for?

Would you expect overriding _Buffer's __copy__ and __deepcopy__ methods be helpful here? as a way to make sure it isn't copied (thus creating two pointers to the same memory object).

I will appreciate keeping this issue open for a while while I investigate and report back, Thanks again!

EDIT:

So far, this is what I've gathered: Our code is implemented as a singleton object who's being accessed from multiple threads. There is a lock that's rarely used, but of course, isn't always used. It doesn't look like the singleton object stores or returns any iptc objects so I guess the issue isn't about sharing objects between threads. I'd imagine iptc isn't thread-safe however, and that's what's causing the issue?

I've also read a bit about the easy submodule, is that thread safe? Is there a recommended approach here (first idea that pops is wrap easy calls with a singleton object that locks on all (or most?) methods.

If there's any documentation about thread safety and common practices here I'd love to read it before rewriting the entire thing from scratch. Unfortunately whoever wrote our code seemed to ignore any thread safety and memory management issues because "its python, you don't need to worry about that type of thing".

Thanks for reading through!

ldx commented 3 years ago

The best is if you can do all iptables operations in one process and thread. In Python you can use a queue or something similar to send messages to this thread e.g. "insert a rule dropping packets going to 10.1.2.3/32", instead of calling a function directly.

The easy submodule is great to simplify things and reduce boilerplate code significantly. But it does not do any locking in itself.

Feel free to post an update if you can reproduce the problem or you'd like to vet something.

nirizr commented 3 years ago

Thanks! It'll take a while before we get someone to spend time on this apparently, but I think I know what we should be doing to avoid the issues we're having. Thank you so much!

I think i'll close this issue for now to avoid cluttering your issue tracker and come back here if we need any further assistance.