[PATCH ulogd2 v2] pcap: prevent crashes when output `FILE *` is null

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
+
+	now = time(NULL);
+
+	if (now < pi->last_outfile_check + pi->next_outfile_check_delta)
+		return -EAGAIN;
+
+	ret = append_create_outfile(upi);
+
+	if (!ret) {
+		pi->last_outfile_check = 0;
+		pi->next_outfile_check_delta = 1;
+		return 0;
+	}
+
+	pi->last_outfile_check = now;
+	if (pi->next_outfile_check_delta <= MAX_OUTFILE_CHECK_DELTA / 2)
+		pi->next_outfile_check_delta *= 2;
+
+	return ret;
+}
+
 static int interp_pcap(struct ulogd_pluginstance *upi)
 {
 	struct pcap_instance *pi = (struct pcap_instance *) &upi->private;
 	struct ulogd_key *res = upi->input.keys;
 	struct pcap_sf_pkthdr pchdr;
 
+	switch (check_outfile(upi)) {
+	case 0:
+		break;
+	case -EAGAIN:
+		return ULOGD_IRET_OK;
+	default:
+		return ULOGD_IRET_ERR;
+	}
+
 	pchdr.caplen = ikey_get_u32(&res[1]);
 
 	/* Try to set the len field correctly, if we know the protocol. */
@@ -275,6 +319,11 @@ static int configure_pcap(struct ulogd_pluginstance *upi,
 
 static int start_pcap(struct ulogd_pluginstance *upi)
 {
+	struct pcap_instance *pi = (struct pcap_instance *) &upi->private;
+
+	pi->last_outfile_check = 0;
+	pi->next_outfile_check_delta = 1;
+
 	return append_create_outfile(upi);
 }
 
-- 
2.39.0




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux