-
Notifications
You must be signed in to change notification settings - Fork 63
Bug fix for segfault #105
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
Bug fix for segfault #105
Conversation
In read_thread(), in the "Do we need this block?" block, it failed to advance to the next wanted_t when w->end is exactly equal to uend. In the particular case I looked at, this resulted in read_thread() erroneously putting two blocks into the queue (pipeline_split(), read.c:570) instead of one. This resulted in a subsequent crash in tar_read() at read.c:660, where gArWanted was null (when clearly the code does not expect it to be null at that point). My use case: I have several hundred large .tar.xz files (created by pixz). Each archive contains over 200k files. I frequently need to extract a lot of files from each archive, but not all files, only a specific subset. So I am making heavy use of the -x option to pixz.
Don't hold queue mutex while calling malloc/free
Update: I've been using this fix for over 8 months so far with no problems. |
@@ -536,7 +536,7 @@ static void read_thread(void) { | |||
debug("read: skip %llu", iter.block.number_in_file); | |||
continue; | |||
} | |||
for ( ; w && w->end < uend; w = w->next) ; | |||
for ( ; w && w->end <= uend; w = w->next) ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to have <= here together with the >= in the if condition above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm that's start, I think this makes sense.
We run into the same issue on Ubuntu 22.04 with its stock pixz 1.0.7, where It's also unsafe to assume, in @vasi |
We also confirmed the crash on that particular file can be mitigated by:
|
In read_thread(), in the "Do we need this block?" block, it failed
to advance to the next wanted_t when w->end is exactly equal to uend.
In the particular case I looked at, this resulted in read_thread()
erroneously putting two blocks into the queue (pipeline_split(),
read.c:570) instead of one.
This resulted in a subsequent crash in tar_read() at read.c:660,
where gArWanted was null (when clearly the code does not expect it
to be null at that point).
My use case:
I have several hundred large .tar.xz files (created by pixz). Each
archive contains over 200k files. I frequently need to extract a lot
of files from each archive, but not all files, only a specific subset.
So I am making heavy use of the -x option to pixz.