When sasyncd(8) dumps data from the kernel, it does a sysctl() for the size to
alloc,
and a second for the actual data. In between, new data can be added/deleted.
Therefore
the kernel might return less data, than actually told. The case that you get
less/more data
isn't handled properly.
In case of less data, it might happen that the child is working on
uninitialized data.
This might lead to a segmentation fault.
In case of more data, the sasyncd(8) parent process handles it as he didn't
get any data.
This diff rewrites that part, so that we try thrice to get the SAs and SPDs.
If getting the SAs
breaks thrice, we tell the child process, that there's no data. Same for the
SPDs.
The case of less data is now handled properly. The case of more data is either
caught
by the for loop, or allocing twice the size of the data we should originally
get.
That might be helpful on bigger vpn setups, when the loop still doesn't catch
the race.
Tested with 7080 SAs. (For real.)
Index: monitor.c
===================================================================
RCS file: /cvs/src/usr.sbin/sasyncd/monitor.c,v
retrieving revision 1.15
diff -u -r1.15 monitor.c
--- monitor.c 2 Apr 2012 18:56:15 -0000 1.15
+++ monitor.c 4 Sep 2012 13:06:37 -0000
@@ -342,7 +342,7 @@
{
u_int8_t *sadb_buf = 0, *spd_buf = 0;
size_t sadb_buflen = 0, spd_buflen = 0, sz;
- int mib[5];
+ int i, mib[5];
u_int32_t v;
mib[0] = CTL_NET;
@@ -352,59 +352,81 @@
mib[4] = 0; /* Unspec SA type */
/* First, fetch SADB data */
- if (sysctl(mib, sizeof mib / sizeof mib[0], NULL, &sz, NULL, 0) == -1
- || sz == 0) {
- sadb_buflen = 0;
- goto try_spd;
- }
-
- sadb_buflen = sz;
- if ((sadb_buf = malloc(sadb_buflen)) == NULL) {
- log_err("m_priv_pfkey_snap: malloc");
- sadb_buflen = 0;
- goto out;
- }
-
- if (sysctl(mib, sizeof mib / sizeof mib[0], sadb_buf, &sz, NULL, 0)
- == -1) {
- log_err("m_priv_pfkey_snap: sysctl");
- sadb_buflen = 0;
- goto out;
+ for (i = 0; i < 3; i++) {
+ if (sysctl(mib, sizeof mib / sizeof mib[0], NULL, &sz, NULL, 0)
+ == -1)
+ break;
+
+ if (!sz)
+ break;
+
+ /* Try to catch newly added data */
+ sz *= 2;
+
+ if ((sadb_buf = malloc(sz)) == NULL)
+ break;
+
+ if (sysctl(mib, sizeof mib / sizeof mib[0], sadb_buf, &sz,
NULL, 0)
+ == -1) {
+ free(sadb_buf);
+ sadb_buf = 0;
+ /*
+ * If new SAs were added meanwhile and the given buffer
is
+ * too small, retry.
+ */
+ if (errno == ENOMEM)
+ continue;
+ break;
+ }
+
+ sadb_buflen = sz;
+ break;
}
/* Next, fetch SPD data */
- try_spd:
mib[3] = NET_KEY_SPD_DUMP;
- if (sysctl(mib, sizeof mib / sizeof mib[0], NULL, &sz, NULL, 0) == -1
- || sz == 0) {
- spd_buflen = 0;
- goto out;
- }
+ for (i = 0; i < 3; i++) {
+ if (sysctl(mib, sizeof mib / sizeof mib[0], NULL, &sz, NULL, 0)
+ == -1)
+ break;
- spd_buflen = sz;
- if ((spd_buf = malloc(spd_buflen)) == NULL) {
- log_err("m_priv_pfkey_snap: malloc");
- spd_buflen = 0;
- goto out;
- }
+ if (!sz)
+ break;
+
+ /* Try to catch newly added data */
+ sz *= 2;
+
+ if ((spd_buf = malloc(sz)) == NULL)
+ break;
+
+ if (sysctl(mib, sizeof mib / sizeof mib[0], spd_buf, &sz, NULL,
0)
+ == -1) {
+ free(spd_buf);
+ spd_buf = 0;
+ /*
+ * If new SPDs were added meanwhile and the given
buffer is
+ * too small, retry.
+ */
+ if (errno == ENOMEM)
+ continue;
+ break;
+ }
- if (sysctl(mib, sizeof mib / sizeof mib[0], spd_buf, &sz, NULL, 0)
- == -1) {
- log_err("m_priv_pfkey_snap: sysctl");
- spd_buflen = 0;
- goto out;
+ spd_buflen = sz;
+ break;
}
- out:
/* Return SADB data */
v = (u_int32_t)sadb_buflen;
if (m_write(s, &v, sizeof v) == -1) {
log_err("m_priv_pfkey_snap: write");
goto cleanup;
}
- if (m_write(s, sadb_buf, sadb_buflen) == -1)
+ if (m_write(s, sadb_buf, sadb_buflen) == -1) {
log_err("m_priv_pfkey_snap: write");
+ goto cleanup;
+ }
/* Return SPD data */
v = (u_int32_t)spd_buflen;
@@ -412,13 +434,20 @@
log_err("m_priv_pfkey_snap: write");
goto cleanup;
}
- if (m_write(s, spd_buf, spd_buflen) == -1)
+ if (m_write(s, spd_buf, spd_buflen) == -1) {
log_err("m_priv_pfkey_snap: write");
- cleanup:
- memset(sadb_buf, 0, sadb_buflen);
- free(sadb_buf);
- memset(spd_buf, 0, spd_buflen);
- free(spd_buf);
+ goto cleanup;
+ }
+
+cleanup:
+ if (sadb_buf) {
+ memset(sadb_buf, 0, sadb_buflen);
+ free(sadb_buf);
+ }
+ if (spd_buf) {
+ memset(spd_buf, 0, spd_buflen);
+ free(spd_buf);
+ }
}
static int
