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;