473,325 Members | 2,480 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,325 software developers and data experts.

possibly memory leak?

Hello, All!

In this piece of code I get 'segmentation fault' periodically:

....
struct dhcp_hdr {
/* ... various fields here*/
unsigned char options[312];
}
....
struct dhcp_hdr *dhcph;
unsigned char *p, *end, opt;
char *tmp = NULL;
unsigned int len=0;
....
p = dhcph->options + 4;
end = dhcph->options + sizeof(dhcph->options);

tmp = malloc(100);

do {
len = *(++p); /* save option length */
p--;
if ( *p == 190 || *p == 191 ) {
opt = *p;
p += 2;
strncpy(tmp, (char *)p, len);
printf("option %u found, value is %s\n", opt, tmp);

p += len;
}
else
p = (p + 2) + len; /* move pointer to the next tag */
} while ( p != end );
....

Seems like 'p' points to illegal chunk of memory at some time and I'm unable
to find this properly. Where can it be?

With best regards, Roman Mashak. E-mail: mr*@tusur.ru
Nov 15 '05 #1
9 1334
option[0].....option[312]......
|--> p?

Nov 15 '05 #2
option[0].....option[312]......
|--> p?

Nov 15 '05 #3
struct dhcp_hdr {
/* ... various fields here*/
unsigned char options[312];
}
p = dhcph->options + 4;
*(++p),p--,(*p)?(*p,p+=2,p+=len):(p=p+2+len) .
--------------------------------------------
}while( p < end ) trying...

Nov 15 '05 #4
} while ( p != end );
------- }while( p < end ); try to this ?


Nov 15 '05 #5
Roman Mashak wrote:
Hello, All!

In this piece of code I get 'segmentation fault' periodically: [...] do {
len = *(++p); /* save option length */
p--; len = *p; if ( *p == 190 || *p == 191 ) {
opt = *p;
p += 2; assert( (p+len) <= end); // include <assert.h>, you could also
// do a check
if ((p+len) <= end)
{ strncpy(tmp, (char *)p, len);
printf("option %u found, value is %s\n", opt, tmp);

p += len; }
else
{
printf("Not enough buffer\n");
break;
} }
else
p = (p + 2) + len; /* move pointer to the next tag */
} while ( p != end );

------------^
(p < end)
[...]

1) Bounds check p != end check may be wrong and should be changed to
(p < end)
Unless you're always guaranteed that p stops at end, not just passes
by it. (I am always paranoid and would prefer the first one.)

2) Do the bounds check just after the pointer modification, or
just before the access.. (I always do the first one)
p += ...; // after p+=2, p+= len, p += (2+len);
if (p >= end) break; // do this check.
Nov 15 '05 #6
Le 15-11-2005, Roman Mashak <mr*@tusur.ru> a écrit*:
struct dhcp_hdr {
/* ... various fields here*/
unsigned char options[312];
}
...
struct dhcp_hdr *dhcph;
unsigned char *p, *end, opt;
char *tmp = NULL;
unsigned int len=0;
...
p = dhcph->options + 4;
end = dhcph->options + sizeof(dhcph->options);

tmp = malloc(100);
Why ? You are sure that this is sufficient ?
do {
len = *(++p); /* save option length */
p--;
Why not
len=p[1]
or
len=*(p+1)
if ( *p == 190 || *p == 191 ) {
Some magic number I assume.
opt = *p;
p += 2;
strncpy(tmp, (char *)p, len);
You are sure that the option is nul-terminated and that
strlen(p)<len ?
printf("option %u found, value is %s\n", opt, tmp);

p += len;
}
else
p = (p + 2) + len; /* move pointer to the next tag */
} while ( p != end );
...

Seems like 'p' points to illegal chunk of memory at some time and I'm unable
to find this properly. Where can it be?


Its impossible to known without any information on the value of
the unsigned chars in the dhcph->options array. Perhaps could you
add some assertions in your code, like
assert(p < end );
assert(strlen(p)<len);
assert(len<100);
etc...

Marc Boyer
Nov 15 '05 #7
Hello, Marc!
You wrote on Tue, 15 Nov 2005 08:47:50 +0000 (UTC):

[skip]
??>> ...
??>>
??>> Seems like 'p' points to illegal chunk of memory at some time and I'm
??>> unable to find this properly. Where can it be?

MB> Its impossible to known without any information on the value of
MB> the unsigned chars in the dhcph->options array. Perhaps could you
MB> add some assertions in your code, like
MB> assert(p < end );
MB> assert(strlen(p)<len);
MB> assert(len<100);
MB> etc...
Thanks to everybody for replies. The problem was with the bound checking in
'while' loop:

while (p != end)

changed into

while ( p < end )

ans also a few clumsy spots were fixed.

With best regards, Roman Mashak. E-mail: mr*@tusur.ru
Nov 15 '05 #8
Roman Mashak wrote:
In this piece of code I get 'segmentation fault' periodically:

...
struct dhcp_hdr {
/* ... various fields here*/
unsigned char options[312];
}
...
struct dhcp_hdr *dhcph;
unsigned char *p, *end, opt;
char *tmp = NULL;
unsigned int len=0;
...
p = dhcph->options + 4;
end = dhcph->options + sizeof(dhcph->options);


At this point dhcph is either a null pointer or undefined and the
results of assigning p and end are undefined. You need to assign a
value to dhcph.

--
Thad

Nov 15 '05 #9
Roman Mashak wrote:
Hello, All!

In this piece of code I get 'segmentation fault' periodically:
Your code is not very robust when faced with malicious data.
struct dhcp_hdr {
/* ... various fields here*/
unsigned char options[312];
}
struct dhcp_hdr *dhcph;
unsigned char *p, *end, opt;
char *tmp = NULL;
unsigned int len=0;
...
p = dhcph->options + 4;
end = dhcph->options + sizeof(dhcph->options);

tmp = malloc(100);

do {
len = *(++p);
p--;
Please replace those two lines with:
len = p[1];
if ( *p == 190 || *p == 191 ) {
opt = *p;
p += 2;
strncpy(tmp, (char *)p, len);
If (len > 100) then you cause a buffer overflow. Check this first.
If (end - p) < len, and there isn't a 0-terminator in there,
then you read past the end of the buffer. Fix this.

strncpy is generally a poor function to use, because once you
have added enough code to stop overflows, it would have been
easier to just do a memcpy or some other copy function.
printf("option %u found, value is %s\n", opt, tmp);

p += len;
}
else
p = (p + 2) + len; /* move pointer to the next tag */
Again, check that you aren't pointing p past the end here.
If you correctly checked it earlier, then it wouldn't be
necessary to re-check. I suggest that when you read 'len',
you check that it's < 100 and that 2 + len < (end - p), and
if not, assume the rest of the input is corrupt.
} while ( p != end );


You commented that changing this to (p < end) fixed the
problem. This is the ambulance at the bottom of the cliff.
You would still be pointing past the end of your buffer,
which is a no-no. It's essential you fix the underlying
cause of the overflow.

Nov 15 '05 #10

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

8
by: ranjeet.gupta | last post by:
Dear All Is the Root Cause of the Memory corruption is the Memory leak, ?? suppose If in the code there is Memory leak, Do this may lead to the Memory Corruption while executing the program ? ...
17
by: José Joye | last post by:
Hi, I have implemented a Service that is responsible for getting messages from a MS MQ located on a remote machine. I'm getting memory leak from time to time (???). In some situation, it is...
4
by: Don Nell | last post by:
Hello Why is there a memory leak when this code is executed. for(;;) { ManagementScope scope = new ManagementScope(); scope.Options.Username="username"; scope.Options.Password="password";...
20
by: jeevankodali | last post by:
Hi I have an .Net application which processes thousands of Xml nodes each day and for each node I am using around 30-40 Regex matches to see if they satisfy some conditions are not. These Regex...
23
by: James | last post by:
The following code will create memory leaks!!! using System; using System.Diagnostics; using System.Data; using System.Data.SqlClient; namespace MemoryLeak
8
by: Adrian | last post by:
Hi I have a JS program that runs localy (under IE6 only) on a PC but it has a memory leak (probably the known MS one!) What applications are there that I could use to look at the memory usage of...
7
by: Salvador | last post by:
Hi, I am using WMI to gather information about different computers (using win2K and win 2K3), checking common classes and also WMI load balance. My application runs every 1 minute and reports...
3
by: Jim Land | last post by:
Jack Slocum claims here http://www.jackslocum.com/yui/2006/10/02/3-easy-steps-to-avoid-javascript- memory-leaks/ that "almost every site you visit that uses JavaScript is leaking memory". ...
7
by: Ragnar Agustsson | last post by:
Hi all I have been wandering about the best way to sandbox memory leaks in 3rd party libraries when using them from the .Net framework. I have a 3rd party library, written in C++, that leaks a...
22
by: Peter | last post by:
I am using VS2008. I have a Windows Service application which creates Crystal Reports. This is a multi theaded application which can run several reports at one time. My problem - there is a...
0
by: DolphinDB | last post by:
Tired of spending countless mintues downsampling your data? Look no further! In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
0
by: ryjfgjl | last post by:
ExcelToDatabase: batch import excel into database automatically...
1
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: Vimpel783 | last post by:
Hello! Guys, I found this code on the Internet, but I need to modify it a little. It works well, the problem is this: Data is sent from only one cell, in this case B5, but it is necessary that data...
0
by: jfyes | last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
0
by: ArrayDB | last post by:
The error message I've encountered is; ERROR:root:Error generating model response: exception: access violation writing 0x0000000000005140, which seems to be indicative of an access violation...
1
by: PapaRatzi | last post by:
Hello, I am teaching myself MS Access forms design and Visual Basic. I've created a table to capture a list of Top 30 singles and forms to capture new entries. The final step is a form (unbound)...
1
by: CloudSolutions | last post by:
Introduction: For many beginners and individual users, requiring a credit card and email registration may pose a barrier when starting to use cloud servers. However, some cloud server providers now...
1
by: Defcon1945 | last post by:
I'm trying to learn Python using Pycharm but import shutil doesn't work

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.