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.

Sign up for GitHub

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

Jump to bottom

log-pcap: fix segfault on lz4 compressed pcaps #6875

Closed
wants to merge 6 commits into from
Closed

log-pcap: fix segfault on lz4 compressed pcaps #6875

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e2f53ed
Fixed segfault on using lz4 compressed pcaps if you are unable to wri…
oxagast Jan 27, 2022
30960b2
Run the clang 9 formatting script on code fix for lz4 pcap segfault.
oxagast Jan 27, 2022
ef7e4a5
Squashed commit of the following:
oxagast Jan 28, 2022
4765356
Merge github.com:oxagast/suricata into HEAD
oxagast Jan 28, 2022
c2bb99b
Merge https://github.com/OISF/suricata
oxagast Jan 30, 2022
4fa3b55
Edited old code out of the comment.
oxagast Jan 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/log-pcap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,12 @@ static int PcapLogOpenHandles(PcapLogData *pl, const Packet *p)
}
comp->file = fopen(pl->filename, "w");
if (comp->file == NULL) {
SCLogError(SC_ERR_OPENING_FILE,
"Error opening file for compressed output: %s",
strerror(errno));
/* took this out so that it won't print twice, but still would function
same SCLogError(SC_ERR_OPENING_FILE, "Error opening file for compressed output:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if removing output please just remove it instead of leaving it as a comment. It is fine to have a comment explaining why there is no error message here

%s", strerror(errno));
*/
return TM_ECODE_FAILED;
}

uint64_t bytes_written = LZ4F_compressBegin(comp->lz4f_context,
comp->buffer, comp->buffer_size, NULL);
if (LZ4F_isError(bytes_written)) {
Expand Down Expand Up @@ -576,6 +576,12 @@ static int PcapLog (ThreadVars *t, void *thread_data, const Packet *p)
SCLogError(SC_ERR_FSEEK, "fseek failed: %s", strerror(errno));
return TM_ECODE_FAILED;
}
comp->file = fopen(pl->filename, "w");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a check for comp->file here, which suggests it might be already open and this would leak the previous descriptor. We're in the packet path here, so we would call this for each packet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If comp->file is null here, I think it means we didn't properly error check something during setup. I think the issue should be handled during setup/initialization (or file rotation)

if (comp->file == NULL) {
SCLogError(SC_ERR_OPENING_FILE, "Error opening file for compressed output: %s",
strerror(errno));
return TM_ECODE_FAILED;
}
if (fwrite(comp->buffer, 1, out_size, comp->file) < out_size) {
SCLogError(SC_ERR_FWRITE, "fwrite failed: %s", strerror(errno));
return TM_ECODE_FAILED;
Expand Down