diff --git a/qa/1927 b/qa/1927
new file mode 100755
index 000000000..46afa9509
--- /dev/null
+++ b/qa/1927
@@ -0,0 +1,88 @@
+#!/bin/sh
+# PCP QA Test No. 1927
+# Exercise the sockets PMDA Install/Remove and string metric bug.
+#
+# Copyright (c) 2022 Red Hat. All Rights Reserved.
+#
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+# get standard environment, filters and checks
+. ./common.product
+. ./common.filter
+. ./common.check
+
+[ -f $PCP_PMDAS_DIR/sockets/pmdasockets ] || _notrun "sockets pmda not installed"
+
+_cleanup()
+{
+ cd $here
+ $sudo rm -rf $tmp $tmp.*
+}
+
+status=0 # success is the default!
+$sudo rm -rf $tmp $tmp.* $seq.full
+
+_filter_sockets()
+{
+ grep -v 'No value(s) available'
+}
+
+pmdasockets_remove()
+{
+ echo
+ echo "=== remove sockets agent ==="
+ $sudo ./Remove >$tmp.out 2>&1
+ _filter_pmda_remove <$tmp.out
+}
+
+pmdasockets_install()
+{
+ # start from known starting points
+ cd $PCP_PMDAS_DIR/sockets
+ $sudo ./Remove >/dev/null 2>&1
+
+ echo
+ echo "=== sockets agent installation ==="
+ $sudo ./Install </dev/null >$tmp.out 2>&1
+ cat $tmp.out >>$here/$seq.full
+ # Check sockets metrics have appeared ... X metrics and Y values
+ _filter_pmda_install <$tmp.out \
+ | sed \
+ -e 's/[0-9][0-9]* warnings, //' \
+ | $PCP_AWK_PROG '
+/Check network.persocket metrics have appeared/ {
+ if ($7 >= 50 && $7 <= 99) $7 = "X"
+ if ($10 >= 0) $10 = "Y"
+ }
+ { print }'
+}
+
+_prepare_pmda sockets
+# note: _restore_auto_restart pmcd done in _cleanup_pmda()
+trap "_cleanup_pmda sockets; exit \$status" 0 1 2 3 15
+
+_stop_auto_restart pmcd
+
+# real QA test starts here
+pmdasockets_install
+
+# pmcd should have been started by the Install process - check
+if pminfo -v network.persocket > $tmp.info 2> $tmp.err
+then
+ :
+else
+ echo "... failed! ... here is the Install log ..."
+ cat $tmp.out
+fi
+cat $tmp.info $tmp.err | _filter_sockets
+
+echo "Check the values for v6only metric are 0 or 1 ..."
+pminfo -f network.persocket.v6only | egrep -v 'value [01]$' | sed -e '/^$/d'
+
+pmdasockets_remove
+status=0
+
+# success, all done
+exit
diff --git a/qa/1927.out b/qa/1927.out
new file mode 100644
index 000000000..2ae4385fd
--- /dev/null
+++ b/qa/1927.out
@@ -0,0 +1,17 @@
+QA output created by 1927
+
+=== sockets agent installation ===
+Updating the Performance Metrics Name Space (PMNS) ...
+Terminate PMDA if already installed ...
+[...install files, make output...]
+Updating the PMCD control file, and notifying PMCD ...
+Check network.persocket metrics have appeared ... X metrics and Y values
+Check the values for v6only metric are 0 or 1 ...
+network.persocket.v6only
+
+=== remove sockets agent ===
+Culling the Performance Metrics Name Space ...
+network.persocket ... done
+Updating the PMCD control file, and notifying PMCD ...
+[...removing files...]
+Check network.persocket metrics have gone away ... OK
diff --git a/qa/group b/qa/group
index acfc5d208..846c0c4bd 100644
--- a/qa/group
+++ b/qa/group
@@ -1967,6 +1967,7 @@ x11
1901 pmlogger local
1902 help local
1914 atop local
+1927 pmda.sockets local
1937 pmlogrewrite pmda.xfs local
1955 libpcp pmda pmda.pmcd local
1956 pmda.linux pmcd local
diff --git a/src/pmdas/linux_sockets/pmda.c b/src/pmdas/linux_sockets/pmda.c
index d10eacf29..5a3018d8a 100644
--- a/src/pmdas/linux_sockets/pmda.c
+++ b/src/pmdas/linux_sockets/pmda.c
@@ -1,7 +1,7 @@
/*
* Sockets PMDA
*
- * Copyright (c) 2021 Red Hat.
+ * Copyright (c) 2021-2022 Red Hat.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@@ -14,6 +14,7 @@
* for more details.
*/
+#include <ctype.h>
#include "pmapi.h"
#include "pmda.h"
@@ -147,6 +148,31 @@ sockets_fetchCallBack(pmdaMetric *metric, unsigned int inst, pmAtomValue *atom)
return PMDA_FETCH_STATIC;
}
+/*
+ * Restrict the allowed filter strings to only limited special
+ * characters (open and close brackets - everthing else can be
+ * done with alphanumerics) to limit any attack surface here.
+ * The ss filtering language is more complex than we ever want
+ * to be attempting to parse ourself, so we leave that side of
+ * things to the ss command itself.
+ */
+int
+sockets_check_filter(const char *string)
+{
+ const char *p;
+
+ for (p = string; *p; p++) {
+ if (isspace(*p))
+ continue;
+ if (isalnum(*p))
+ continue;
+ if (*p == '(' || *p == ')')
+ continue;
+ return 0; /* disallow */
+ }
+ return 1;
+}
+
static int
sockets_store(pmResult *result, pmdaExt *pmda)
{
@@ -165,9 +191,14 @@ sockets_store(pmResult *result, pmdaExt *pmda)
case 0: /* network.persocket.filter */
if ((sts = pmExtractValue(vsp->valfmt, &vsp->vlist[0],
PM_TYPE_STRING, &av, PM_TYPE_STRING)) >= 0) {
+ if (sockets_check_filter(av.cp)) {
+ sts = PM_ERR_BADSTORE;
+ free(av.cp);
+ break;
+ }
if (ss_filter)
free(ss_filter);
- ss_filter = av.cp; /* TODO filter syntax check */
+ ss_filter = av.cp;
}
break;
default:
diff --git a/src/pmdas/linux_sockets/ss_parse.c b/src/pmdas/linux_sockets/ss_parse.c
index 94c5e16e9..9f3afc691 100644
--- a/src/pmdas/linux_sockets/ss_parse.c
+++ b/src/pmdas/linux_sockets/ss_parse.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2021 Red Hat.
+ * Copyright (c) 2021-2022 Red Hat.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@@ -21,65 +21,70 @@ static ss_stats_t ss_p;
/* boolean value with no separate value, default 0 */
#define PM_TYPE_BOOL (PM_TYPE_UNKNOWN-1)
+/* helper macros to extract field address and size */
+#define SSFIELD(str,type,f) {(str), (sizeof(str)-1), type, (&(f)), (sizeof(f))}
+#define SSNULLFIELD(str) {(str), (sizeof(str)-1), PM_TYPE_UNKNOWN, NULL}
+
static struct {
char *field;
int len;
int type;
void *addr;
+ int size;
int found;
} parse_table[] = {
- { "timer:", 6, PM_TYPE_STRING, &ss_p.timer_str },
- { "uid:", 4, PM_TYPE_U32, &ss_p.uid },
- { "ino:", 4, PM_TYPE_64, &ss_p.inode },
- { "sk:", 3, PM_TYPE_U64, &ss_p.sk },
- { "cgroup:", 7, PM_TYPE_STRING, &ss_p.cgroup },
- { "v6only:", 7, PM_TYPE_32, &ss_p.v6only },
- { "--- ", 4, PM_TYPE_UNKNOWN, NULL },
- { "<-> ", 4, PM_TYPE_UNKNOWN, NULL },
- { "--> ", 4, PM_TYPE_UNKNOWN, NULL },
- { "skmem:", 6, PM_TYPE_STRING, &ss_p.skmem_str, },
- { "ts ", 3, PM_TYPE_BOOL, &ss_p.ts },
- { "sack ", 5, PM_TYPE_BOOL, &ss_p.sack },
- { "cubic ", 6, PM_TYPE_BOOL, &ss_p.cubic },
- { "wscale:", 7, PM_TYPE_STRING, &ss_p.wscale_str },
- { "rto:", 4, PM_TYPE_DOUBLE, &ss_p.rto },
- { "rtt:", 4, PM_TYPE_STRING, &ss_p.round_trip_str },
- { "ato:", 4, PM_TYPE_DOUBLE, &ss_p.ato },
- { "backoff:", 8, PM_TYPE_32, &ss_p.backoff },
- { "mss:", 4, PM_TYPE_U32, &ss_p.mss },
- { "pmtu:", 5, PM_TYPE_U32, &ss_p.pmtu },
- { "rcvmss:", 7, PM_TYPE_U32, &ss_p.rcvmss },
- { "advmss:", 7, PM_TYPE_U32, &ss_p.advmss },
- { "cwnd:", 5, PM_TYPE_U32, &ss_p.cwnd },
- { "lost:", 5, PM_TYPE_32, &ss_p.lost },
- { "ssthresh:", 9, PM_TYPE_U32, &ss_p.ssthresh },
- { "bytes_sent:", 11, PM_TYPE_U64, &ss_p.bytes_sent },
- { "bytes_retrans:", 14, PM_TYPE_U64, &ss_p.bytes_retrans },
- { "bytes_acked:", 12, PM_TYPE_U64, &ss_p.bytes_acked },
- { "bytes_received:", 15, PM_TYPE_U64, &ss_p.bytes_received },
- { "segs_out:", 9, PM_TYPE_U32, &ss_p.segs_out },
- { "segs_in:", 8, PM_TYPE_U32, &ss_p.segs_in },
- { "data_segs_out:", 14, PM_TYPE_U32, &ss_p.data_segs_out },
- { "data_segs_in:", 13, PM_TYPE_U32, &ss_p.data_segs_in },
- { "send ", 5, PM_TYPE_DOUBLE, &ss_p.send }, /* no ':' */
- { "lastsnd:", 8, PM_TYPE_U32, &ss_p.lastsnd },
- { "lastrcv:", 8, PM_TYPE_U32, &ss_p.lastrcv },
- { "lastack:", 8, PM_TYPE_U32, &ss_p.lastack },
- { "pacing_rate ", 12, PM_TYPE_DOUBLE, &ss_p.pacing_rate }, /* no ':' */
- { "delivery_rate ", 14, PM_TYPE_DOUBLE, &ss_p.delivery_rate }, /* no ':' */
- { "delivered:", 10, PM_TYPE_U32, &ss_p.delivered },
- { "app_limited ", 12, PM_TYPE_BOOL, &ss_p.app_limited },
- { "reord_seen:", 11, PM_TYPE_32, &ss_p.reord_seen },
- { "busy:", 5, PM_TYPE_U64, &ss_p.busy },
- { "unacked:", 8, PM_TYPE_32, &ss_p.unacked },
- { "rwnd_limited:", 13, PM_TYPE_U64, &ss_p.rwnd_limited },
- { "retrans:", 8, PM_TYPE_STRING, &ss_p.retrans_str },
- { "dsack_dups:", 11, PM_TYPE_U32, &ss_p.dsack_dups },
- { "rcv_rtt:", 8, PM_TYPE_DOUBLE, &ss_p.rcv_rtt },
- { "rcv_space:", 10, PM_TYPE_32, &ss_p.rcv_space },
- { "rcv_ssthresh:", 13, PM_TYPE_32, &ss_p.rcv_ssthresh },
- { "minrtt:", 7, PM_TYPE_DOUBLE, &ss_p.minrtt },
- { "notsent:", 8, PM_TYPE_U32, &ss_p.notsent },
+ SSFIELD("timer:", PM_TYPE_STRING, ss_p.timer_str),
+ SSFIELD("uid:", PM_TYPE_U32, ss_p.uid),
+ SSFIELD("ino:", PM_TYPE_64, ss_p.inode),
+ SSFIELD("sk:", PM_TYPE_U64, ss_p.sk),
+ SSFIELD("cgroup:", PM_TYPE_STRING, ss_p.cgroup),
+ SSFIELD("v6only:", PM_TYPE_32, ss_p.v6only),
+ SSNULLFIELD("--- "),
+ SSNULLFIELD("<-> "),
+ SSNULLFIELD("--> "),
+ SSFIELD("skmem:", PM_TYPE_STRING, ss_p.skmem_str),
+ SSFIELD("ts ", PM_TYPE_BOOL, ss_p.ts),
+ SSFIELD("sack ", PM_TYPE_BOOL, ss_p.sack),
+ SSFIELD("cubic ", PM_TYPE_BOOL, ss_p.cubic),
+ SSFIELD("wscale:", PM_TYPE_STRING, ss_p.wscale_str),
+ SSFIELD("rto:", PM_TYPE_DOUBLE, ss_p.rto),
+ SSFIELD("rtt:", PM_TYPE_STRING, ss_p.round_trip_str),
+ SSFIELD("ato:", PM_TYPE_DOUBLE, ss_p.ato),
+ SSFIELD("backoff:", PM_TYPE_32, ss_p.backoff),
+ SSFIELD("mss:", PM_TYPE_U32, ss_p.mss),
+ SSFIELD("pmtu:", PM_TYPE_U32, ss_p.pmtu),
+ SSFIELD("rcvmss:", PM_TYPE_U32, ss_p.rcvmss),
+ SSFIELD("advmss:", PM_TYPE_U32, ss_p.advmss),
+ SSFIELD("cwnd:", PM_TYPE_U32, ss_p.cwnd),
+ SSFIELD("lost:", PM_TYPE_32, ss_p.lost),
+ SSFIELD("ssthresh:", PM_TYPE_U32, ss_p.ssthresh),
+ SSFIELD("bytes_sent:", PM_TYPE_U64, ss_p.bytes_sent),
+ SSFIELD("bytes_retrans:", PM_TYPE_U64, ss_p.bytes_retrans),
+ SSFIELD("bytes_acked:", PM_TYPE_U64, ss_p.bytes_acked),
+ SSFIELD("bytes_received:", PM_TYPE_U64, ss_p.bytes_received),
+ SSFIELD("segs_out:", PM_TYPE_U32, ss_p.segs_out),
+ SSFIELD("segs_in:", PM_TYPE_U32, ss_p.segs_in),
+ SSFIELD("data_segs_out:", PM_TYPE_U32, ss_p.data_segs_out),
+ SSFIELD("data_segs_in:", PM_TYPE_U32, ss_p.data_segs_in),
+ SSFIELD("send ", PM_TYPE_DOUBLE, ss_p.send), /* no ':' */
+ SSFIELD("lastsnd:", PM_TYPE_U32, ss_p.lastsnd),
+ SSFIELD("lastrcv:", PM_TYPE_U32, ss_p.lastrcv),
+ SSFIELD("lastack:", PM_TYPE_U32, ss_p.lastack),
+ SSFIELD("pacing_rate ", PM_TYPE_DOUBLE, ss_p.pacing_rate), /* no ':' */
+ SSFIELD("delivery_rate ", PM_TYPE_DOUBLE, ss_p.delivery_rate), /* no ':' */
+ SSFIELD("delivered:", PM_TYPE_U32, ss_p.delivered),
+ SSFIELD("app_limited ", PM_TYPE_BOOL, ss_p.app_limited),
+ SSFIELD("reord_seen:", PM_TYPE_32, ss_p.reord_seen),
+ SSFIELD("busy:", PM_TYPE_U64, ss_p.busy),
+ SSFIELD("unacked:", PM_TYPE_32, ss_p.unacked),
+ SSFIELD("rwnd_limited:", PM_TYPE_U64, ss_p.rwnd_limited),
+ SSFIELD("retrans:", PM_TYPE_STRING, ss_p.retrans_str),
+ SSFIELD("dsack_dups:", PM_TYPE_U32, ss_p.dsack_dups),
+ SSFIELD("rcv_rtt:", PM_TYPE_DOUBLE, ss_p.rcv_rtt),
+ SSFIELD("rcv_space:", PM_TYPE_32, ss_p.rcv_space),
+ SSFIELD("rcv_ssthresh:", PM_TYPE_32, ss_p.rcv_ssthresh),
+ SSFIELD("minrtt:", PM_TYPE_DOUBLE, ss_p.minrtt),
+ SSFIELD("notsent:", PM_TYPE_U32, ss_p.notsent),
{ NULL }
};
@@ -225,8 +230,11 @@ ss_parse(char *line, int has_state_field, ss_stats_t *ss)
if (*p == '(')
p++;
r = (char *)parse_table[i].addr;
- for (s=p; *s && *s != ' ' && *s != '\n' && *s != ')'; s++)
- *r++ = *s; /* TODO check r len */
+ for (s=p; *s && *s != ' ' && *s != '\n' && *s != ')'; s++) {
+ *r++ = *s;
+ if (r - (char *)parse_table[i].addr >= parse_table[i].size - 1)
+ break;
+ }
*r = '\0';
break;
case PM_TYPE_32:
diff --git a/src/pmdas/linux_sockets/ss_stats.h b/src/pmdas/linux_sockets/ss_stats.h
index 183db5afa..009a00cd9 100644
--- a/src/pmdas/linux_sockets/ss_stats.h
+++ b/src/pmdas/linux_sockets/ss_stats.h
@@ -1,11 +1,11 @@
/*
- * Copyright (c) 2021 Red Hat.
- *
+ * Copyright (c) 2021-2022 Red Hat.
+ *
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
* option) any later version.
- *
+ *
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
@@ -26,7 +26,7 @@ typedef struct ss_stats {
__int32_t timer_retrans;
__uint32_t uid;
__uint64_t sk;
- char cgroup[64];
+ char cgroup[128];
__int32_t v6only;
char skmem_str[64];
__int32_t skmem_rmem_alloc;