Re: [Openslp-devel] Segfault Seen In Latest Openslp
By default the DA detection is set to true using the following parameters in /etc/slp.conf net.slp.passiveDADetection net.slp.passiveDADetection You may want to try setting these to false to avoid automatic DA detection. Regards -KPRajesh On Mon, Aug 8, 2011 at 10:13 AM, Varun Chandramohan var...@linux.vnet.ibm.com wrote: While we are on the discussion, i noticed that my slpd daemon configured as SA is taking registrations from outside (there are DAs in the n/w). That sounds wrong! I think SA should not take registrations from DA's right? Regards, Varun On Wednesday, August 03, 2011 02:58:57 pm Hird Matthew wrote: Varun How many nodes do you have in your system? also, how long are your scope names? I've found a bug in slpd_process.c which is related to these areas and causes the daemon to segfault. As soon as we hit 50 DAs, all the SLP daemons seg faulted at the same time. I have a fix which I'll submit shortly but you could just up that buffer to 8192 as a temporary test. What happens is that when a search is performed on a node, the slptool asks the daemon for a list of it's known scopes via a 'backdoor'. the daemon builds up a list of known DAs from it's database and returns them all in the form of a DAAdvert. This will vary in size according to your scope name and IP address but each advert is in the order of 70-100 bytes, assuming a scope name of less than 10 chars. It puts it's own local loopback address in at the start of the list and a terminating DAAdvert at the end so you're down to ~3936 bytes straight off. Assuming a DAAdvert size of ~80 bytes, means you can only fit 49/50 DAs in the list. Obviously, if you have a very scope name, even less. could this be the problem you're seeing? Like I said, I'll fix the code to dynamically size the buffer to something appropriate and submit the patch soon. cheers Matt _ From: Nick Wagner [mailto:ne...@wingedbeast.org] Sent: 02 August 2011 03:29 To: Richard Morrell Cc: openslp-devel@lists.sourceforge.net Subject: Re: [Openslp-devel] Segfault Seen In Latest Openslp 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 mailto: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
Re: [Openslp-devel] Segfault Seen In Latest Openslp
Varun, That's correct. SA's should only take registrations from the local host. John -Original Message- From: Varun Chandramohan [mailto:var...@linux.vnet.ibm.com] Sent: Sunday, August 07, 2011 10:43 PM To: openslp-devel@lists.sourceforge.net Subject: Re: [Openslp-devel] Segfault Seen In Latest Openslp While we are on the discussion, i noticed that my slpd daemon configured as SA is taking registrations from outside (there are DAs in the n/w). That sounds wrong! I think SA should not take registrations from DA's right? Regards, Varun On Wednesday, August 03, 2011 02:58:57 pm Hird Matthew wrote: Varun How many nodes do you have in your system? also, how long are your scope names? I've found a bug in slpd_process.c which is related to these areas and causes the daemon to segfault. As soon as we hit 50 DAs, all the SLP daemons seg faulted at the same time. I have a fix which I'll submit shortly but you could just up that buffer to 8192 as a temporary test. What happens is that when a search is performed on a node, the slptool asks the daemon for a list of it's known scopes via a 'backdoor'. the daemon builds up a list of known DAs from it's database and returns them all in the form of a DAAdvert. This will vary in size according to your scope name and IP address but each advert is in the order of 70-100 bytes, assuming a scope name of less than 10 chars. It puts it's own local loopback address in at the start of the list and a terminating DAAdvert at the end so you're down to ~3936 bytes straight off. Assuming a DAAdvert size of ~80 bytes, means you can only fit 49/50 DAs in the list. Obviously, if you have a very scope name, even less. could this be the problem you're seeing? Like I said, I'll fix the code to dynamically size the buffer to something appropriate and submit the patch soon. cheers Matt _ From: Nick Wagner [mailto:ne...@wingedbeast.org] Sent: 02 August 2011 03:29 To: Richard Morrell Cc: openslp-devel@lists.sourceforge.net Subject: Re: [Openslp-devel] Segfault Seen In Latest Openslp 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 mailto: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
Re: [Openslp-devel] Segfault Seen In Latest Openslp
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
Re: [Openslp-devel] Segfault Seen In Latest Openslp
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.comwrote:
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
Re: [Openslp-devel] Segfault Seen In Latest Openslp
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
On Wed, Jul 27, 2011 at 1:27 AM, Varun Chandramohan
var...@linux.vnet.ibm.com 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@lists.sourceforge.net
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@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openslp-devel
