On Mon, Apr 1, 2013 at 3:48 PM, Zdenek Styblik <[email protected]> wrote:
> On Fri, Mar 22, 2013 at 1:17 AM, Dan Gora <[email protected]> wrote:
>> If the offset was greater than 127, then the multi_offset would end
>> up being a negative value, which is not what we want.
>>
>
> Dan,
>
> I actually don't understand this patch. Please, can you dumb it down for me?
> Also, given your description, I actually don't think problem has been
> fixed. Shouldn't there, somewhere, be a check whether offset is less
> than 127? Because what you've described is integer *flow going on.
Here's the patched code:
FILE * input_file;
unsigned char data;
unsigned char last_record = 0;
unsigned int multi_offset;
int record_count = 0;
input_file = fopen ( filename, "r");
if ( input_file == NULL ){
lprintf(LOG_ERR, "File: '%s' is not found", filename);
return ERROR_STATUS;
}
fseek ( input_file, START_DATA_OFFSET, SEEK_SET );
fread ( &data, 1, 1, input_file );
if ( data <= 0 ){
lprintf(LOG_NOTICE, "There is no multi record in the file %s\n",
filename);
fclose( input_file );
return ERROR_STATUS;
}
/*the offset value is in multiple of 8 bytes.*/
multi_offset = data * 8;
if ( verbose == LOG_DEBUG ) {
printf( "start multi offset = 0x%02lx\n", multi_offset );
}
fseek ( input_file, multi_offset, SEEK_SET );
while ( !feof( input_file ) ) {
*list_record = malloc ( sizeof (struct ipmi_ek_multi_header) );
fread ( &(*list_record)->header, START_DATA_OFFSET, 1,
input_file);
if ( (*list_record)->header.len == 0 ){
record_count++;
continue;
}
So, 'data' is 1 byte long and it's reading an offset field. The
offset cannot be negative, but it *could* (I guess, in theory, but it
probably never is in practice) be greater than 127. Since the offset
is always positive you want 'data' and 'multi_offset' to be positive,
so we should use unsigned types for these. If 'data' was 128 or more,
it would have yielded a negative offset for the fseek in "fseek (
input_file, multi_offset, SEEK_SET );"
I didn't actually see this bug, but it looked wrong. The offset
should never be negative, but there's nothing saying that it has to be
less than 128.
thanks
d
------------------------------------------------------------------------------
Own the Future-Intel® Level Up Game Demo Contest 2013
Rise to greatness in Intel's independent game demo contest.
Compete for recognition, cash, and the chance to get your game
on Steam. $5K grand prize plus 10 genre and skill prizes.
Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d
_______________________________________________
Ipmitool-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
