Jeremy Sowden <jeremy@xxxxxxxxxx> wrote: > If ulogd2 receives a signal it will attempt to re-open the pcap output > file. If this fails (because the permissions or ownership have changed > for example), the FILE pointer will be null and when the next packet > comes in, the null pointer will be passed to fwrite and ulogd will > crash. > > Instead, check that the pointer is not null before using it. If it is > null, then periodically attempt to open it again. We only return an > error from interp_pcap on those occasions when we try and fail to open > the output file, in order to avoid spamming the ulogd log-file every > time a packet isn't written. > > Link: https://bugs.launchpad.net/ubuntu/+source/ulogd2/+bug/1429778 > > Signed-off-by: Jeremy Sowden <jeremy@xxxxxxxxxx> > --- > Change since v1: correct subject-prefix. > > output/pcap/ulogd_output_PCAP.c | 49 +++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/output/pcap/ulogd_output_PCAP.c b/output/pcap/ulogd_output_PCAP.c > index e7798f20c8fc..5b2ca64d3393 100644 > --- a/output/pcap/ulogd_output_PCAP.c > +++ b/output/pcap/ulogd_output_PCAP.c > @@ -82,6 +82,8 @@ struct pcap_sf_pkthdr { > #define ULOGD_PCAP_SYNC_DEFAULT 0 > #endif > > +#define MAX_OUTFILE_CHECK_DELTA 64 > + > #define NIPQUAD(addr) \ > ((unsigned char *)&addr)[0], \ > ((unsigned char *)&addr)[1], \ > @@ -107,6 +109,7 @@ static struct config_keyset pcap_kset = { > }; > > struct pcap_instance { > + time_t last_outfile_check, next_outfile_check_delta; > FILE *of; > }; > > @@ -142,12 +145,53 @@ static struct ulogd_key pcap_keys[INTR_IDS] = { > > #define GET_FLAGS(res, x) (res[x].u.source->flags) > > +static int append_create_outfile(struct ulogd_pluginstance *); > + > +static int > +check_outfile(struct ulogd_pluginstance *upi) > +{ > + struct pcap_instance *pi = (struct pcap_instance *) &upi->private; > + time_t now; > + int ret; > + > + if (pi->of) > + return 0; I think its better to fix this at the source, i.e. in signal_handler_task(). It should probably *first* try to open the file, and only close the old one if that worked. Does that make sense to you?
