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 $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 #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;