pcp
[Top] [All Lists]

new pdubuf vs. qa/367

To: pcp developers <pcp@xxxxxxxxxxx>
Subject: new pdubuf vs. qa/367
From: "Frank Ch. Eigler" <fche@xxxxxxxxxx>
Date: Sun, 8 Mar 2015 12:17:56 -0400
Delivered-to: pcp@xxxxxxxxxxx
User-agent: Mutt/1.4.2.2i
Hi -

Round 2.  See libpcp/src/p_pmns.c SendNameReq() vs. qa/367.  The four
or so pducheck subtests that use the SendNameReq helper function fail
with the new pdubuf code on fche/multithread (PMNS_CHILD, ...).  The
reason is that this function (alone?) adds uninitialized padding after
outgoing PDUbuf payload (the name string) to round up the PDU to the
sizeof(int).  The 367 test case is sensitive to the content of that
padding.  (The test case embeds in it assumptions about what that
particular pdubuf had in its previous life.)

Some questions.  Why do we pad at all?  Should we fix qa/367 to stop
asserting padding content?  Or teach SendNameReq to initialize all
that padding (see below for that)?  Or have __pmFindPDU() memset-0 the
thing?  Letting the uninitialized padding travel would be a small
security problem, if a pmcd were tricked into leaking information from
other clients.

- FChE


diff --git a/src/libpcp/src/p_pmns.c b/src/libpcp/src/p_pmns.c
index 8a907533b63d..683845252bdf 100644
--- a/src/libpcp/src/p_pmns.c
+++ b/src/libpcp/src/p_pmns.c
@@ -462,6 +462,7 @@ SendNameReq(int fd, int from, const char *name, int 
pdu_type, int subtype)
     int                namelen;
     int                alloc_len; /* length allocated for name */
     int                sts;
+    char        *p;
 
 #ifdef PCP_DEBUG
     if (pmDebug & DBG_TRACE_PMNS) {
@@ -483,6 +484,9 @@ SendNameReq(int fd, int from, const char *name, int 
pdu_type, int subtype)
     nreq->subtype = htonl(subtype);
     nreq->namelen = htonl(namelen);
     memcpy(&nreq->name[0], name, namelen);
+    /* clear the padding bytes, lest they contain garbage */
+    for (p = & (nreq->name[namelen]); p < & ((char*)nreq)[need]; p++)
+        *p = 0x7e;
 
     sts = __pmXmitPDU(fd, (__pmPDU *)nreq);
     __pmUnpinPDUBuf(nreq);

<Prev in Thread] Current Thread [Next in Thread>