Yes, I didn't spot what you were doing until after I sent the email. I've
never done that trick inside of a union -- I wouldn't think so, but is it
possible some compiler optimization is getting in the way? Varun, what kind
of compiler/environment are you using?
--Nick
On Mon, Aug 1, 2011 at 1:51 PM, Richard Morrell <r.morr...@hotmail.com>wrote:
> This seems to be in the code I added while at Thales.
>
> I left there recently, and am just getting myself set back up to have access
> to
> the source and mailing lists from home rather than work.
>
> Any further information on this ?
>
> See comments below.
>
>
> >On 07/28/2011 03:40 AM, Varun Chandramohan wrote:
> >> On Thursday, July 28, 2011 12:51:47 am Nick Wagner wrote:
> >>> The SLPDPredicateTreeNode structure does seem to have a problem. If we
> >>> left
> >>> the larger-sized array, guards should still be added to ensure that
> >>> neither
> >>> strncpy will step off the end of the member 'storage'. Others may have
> >>> differing opinions, but I'd change storage to be a char* and alloc it to
> >>> the
> >>> right size (tag_len + value_len + terminating nulls), as long as we make
> >>> sure that it's freed appropriately. We're allocating all the nodes in the
> >>> tree anyway.
> >>>
> >>> --Nick
> >>>
> >> I think we can do that. If all are ok with it, can we make this change?
> >It seems that the extra space for the attributes is already taken care
> >of in allocating the memory for ppNode (slpd_predicate:1645). So Nick's
> >proposed change shouldn't be necessary. But we still need to find out
> >why it doesn't work properly yet...
> >
> Agreed. The assumption was that "storage" was allocated as the last structure
> member at the end of the structure, and is initially two bytes long to allow
> for
> the trailing NUL characters at the end of each of the tag and value strings.
> The
> space allocated is increased by lhs_len+rhs_len to allow for the storage of
> the
> strings. This is so that you don't need to worry about separately allocating
> and
> de-allocating storage for the (usually short) tag and value strings.
>
> It works fine for me using the latest trunk source built on Linux Mint 7
> x86_64. Can we get any more information from the failure ? How repeatable is
> it ? Can you generate a core file and get information like the values of
> lhs_len
> and rhs_len, operator, and *ppNode at the point of failure using gdb ?
>
>
> >Can we be certain that the 'storage' field is actually exactly at the
> >end of the SLPPredicateTreeNode structure?
> Even if it wasn't, it shouldn't cause a segmentation fault, although it would
> overwrite any structure members allocated following it.
>
>
> >
> >BR,
> > Roel
> >>
> >> - Varun
> >>
> >>> On Wed, Jul 27, 2011 at 1:27 AM, Varun Chandramohan<
> >>> varunc@...> wrote:
> >>>
> >>>> **
> >>>>
> >>>> Hi Folks,
> >>>>
> >>>> The test team reported a bug. Iam pasting the bug analysis. They seem
> >>>> to
> >>>> have found the problem as well with a temporary fix. However i they want
> >>>> our
> >>>> opinion on the fix. Please advice.
> >>>>
> >>>> Using the following command:
> >>>>
> >>>> slptool findsrvs service:test$ID "(foo=value1)"
> >>>>
> >>>> this will generat the overflow in createPredicateParseTree doing an
> >>>> strncpy -
> >>>>
> >>>> line 1656 slpd_predicate.c
> >>>>
> >>>> When reading the comments around "operator" it appears it is using the
> >>>> operator
> >>>>
> >>>> 2 characters as a place to copy the attribute name. The attribute name
> >>>> can be
> >>>>
> >>>> very large. This is the code:
> >>>>
> >>>> /* Finished with "operator" now - just use as temporary pointer
> >>>> to assist
> >>>>
> >>>> with copying the
> >>>>
> >>>> * attribute name (lhs) and required value (rhs) into the node
> >>>>
> >>>> */
> >>>>
> >>>> operator = (*ppNode)->nodeBody.comparison.storage;
> >>>>
> >>>> strncpy(operator, lhs, lhs_len);
> >>>>
> >>>> operator[lhs_len] = '\0';
> >>>>
> >>>> operator is now the pointer of "storage" in:
> >>>>
> >>>> slpd_predicate.h
> >>>>
> >>>> typedef struct __SLPDPredicateTreeNode
> >>>>
> >>>> {
> >>>>
> >>>> SLPDPredicateTreeNodeType nodeType;
> >>>>
> >>>> struct __SLPDPredicateTreeNode *next; /* next node in a
> >>>> combination */
> >>>>
> >>>> union {
> >>>>
> >>>> struct __SLPDPredicateLogicalBody
> >>>>
> >>>> {
> >>>>
> >>>> struct __SLPDPredicateTreeNode *first;
> >>>>
> >>>> } logical;
> >>>>
> >>>> struct __SLPDPredicateComparisonBody
> >>>>
> >>>> {
> >>>>
> >>>> size_t tag_len;
> >>>>
> >>>> char *tag_str;
> >>>>
> >>>> size_t value_len;
> >>>>
> >>>> char *value_str;
> >>>>
> >>>> char storage[2];
> >>>>
> >>>> } comparison;
> >>>>
> >>>> } nodeBody;
> >>>>
> >>>> } SLPDPredicateTreeNode;
> >>>>
> >>>> Copying of attributes onto 2char array fails though doesn't fail in
> >>>> older
> >>>>
> >>>> builds so I am not sure if build options or strictness has changed.
> >>>>
> >>>> Since there were no cleanup routines if the pointer was malloced, I
> >>>> just
> >>>>
> >>>> increased --> storage[200] and the testcase runs without fail and the
> >>>> area will
> >>>>
> >>>> get freed with the structure.
> >>>>
> >>>> Is this the best way out?
> >>>>
> >>>> Regards,
> >>>>
> >>>> Varun
> >>>>
> >>>>
> >>>>
> >>>> ------------------------------------------------------------------------------
> >>>> Got Input? Slashdot Needs You.
> >>>> Take our quick survey online. Come on, we don't ask for help often.
> >>>> Plus, you'll get a chance to win $100 to spend on ThinkGeek.
> >>>> http://p.sf.net/sfu/slashdot-survey
> >>>> _______________________________________________
> >>>> Openslp-devel mailing list
> >>>> Openslp-devel@...
>
> >>>> https://lists.sourceforge.net/lists/listinfo/openslp-devel
> >>>>
> >>>>
> >> ------------------------------------------------------------------------------
> >> Got Input? Slashdot Needs You.
> >> Take our quick survey online. Come on, we don't ask for help often.
> >> Plus, you'll get a chance to win $100 to spend on ThinkGeek.
> >> http://p.sf.net/sfu/slashdot-survey
> >> _______________________________________________
> >> Openslp-devel mailing list
> >> Openslp-devel@...
>
> >> https://lists.sourceforge.net/lists/listinfo/openslp-devel
>
>
>
> ------------------------------------------------------------------------------
> BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
> The must-attend event for mobile developers. Connect with experts.
> Get tools for creating Super Apps. See the latest technologies.
> Sessions, hands-on labs, demos & much more. Register early & save!
> http://p.sf.net/sfu/rim-blackberry-1
> _______________________________________________
> Openslp-devel mailing list
> Openslp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openslp-devel
>
>
------------------------------------------------------------------------------
BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
The must-attend event for mobile developers. Connect with experts.
Get tools for creating Super Apps. See the latest technologies.
Sessions, hands-on labs, demos & much more. Register early & save!
http://p.sf.net/sfu/rim-blackberry-1
_______________________________________________
Openslp-devel mailing list
Openslp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openslp-devel
