pcp
[Top] [All Lists]

RE: [pcp] new pdubuf vs. qa/367

To: "'Frank Ch. Eigler'" <fche@xxxxxxxxxx>, "'pcp developers'" <pcp@xxxxxxxxxxx>
Subject: RE: [pcp] new pdubuf vs. qa/367
From: "Ken McDonell" <kenj@xxxxxxxxxxxxxxxx>
Date: Mon, 9 Mar 2015 07:46:29 +1100
Delivered-to: pcp@xxxxxxxxxxx
In-reply-to: <20150308161756.GH27936@xxxxxxxxxx>
References: <20150308161756.GH27936@xxxxxxxxxx>
Thread-index: AQE9ewNiTDULd15mJHvekeQDpwfmyJ44yCDw
> -----Original Message-----
> From: pcp-bounces@xxxxxxxxxxx [mailto:pcp-bounces@xxxxxxxxxxx] On Behalf
> Of Frank Ch. Eigler
> Sent: Monday, 9 March 2015 3:18 AM
> To: pcp developers
> Subject: [pcp] new pdubuf vs. qa/367
> 
> 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.

I am a little confused Frank.

In the original code the pdu buffer needs to be a multiple of sizeo(__pmPDU)
... this assumption is enforced in __pmFindPDUBuf() because PDU_CHUNK is a
multiple of sizeof(__pmPDU) and required in __pmXmitPDU() where tail end
buffer initialization already happens.

p_pmns.c is special because _all_ of the names are padded to a __pmPDU
boundary and prefixed with a length field.  This produces word aligned
access for all the integers in the buffer in the presence of variable sized
strings ... this last name in the buffer is handled the same as the earlier
names in the buffer (if any).  The reason this is not done elsewhere is that
variable length strings are not encoded in a PDU elsewhere (at least this
was the case originally, but the situation may have changed with the
addition of some later PDU types).
 
So I think the simplest change for your new PDU buffer code would be to
round the requested buffer size to be a multiple of sizeof(__pmPDU) bytes. 

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