[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MiNT] Question about memory under MagiC



On Sunday 23 Oct 2011 00:00:41 Jo Even Skarstein wrote:
> On Sat, 2011-10-22 at 23:45 +0200, Jean-François Lemaire wrote:
> > > Your strcpy has overwritten the end of the allocated memory block.
> > 
> > Since when does MiNT with MP accepts writing past the end of the buffer?
> > I know from experience that it does not.
> 
> Because MiNT will allocate blocks in page-sizes when MP is enabled. So
> your allocated block can be as much as 8k-1 bytes larger than you
> requested.
> 
> > > How are you
> > > *sure* that the string fits in the allocated buffer?
> > 
> > char path[PATHSIZE] = {0};
> > size_t size = snprintf(path, sizeof path, "%s%s", string1, string2) + 1;
> > 
> > then size is passed to the function that does the Mxalloc'ing then
> > strcpy() call.
> 
> "The snprintf function returns the number of characters that would have
> been written had n been sufficiently large, not counting the
> terminating null character, or a negative value if an encoding error
> occurred. Thus, the null-terminated output has been completely written
> if and only if the returned value is nonnegative and less than n."

OK, you're right. But I know that the buffer is much larger than the two 
strings combined. And I've used printf to print the size of the dest and 
source in the crashing part and they were equal. So I'm still not totally 
convinced :-) but I will certainly review my code.
 
> So you can't use the return value of snprintf to decide the size of your
> array. You will allocate a too small block, and strcpy will copy the
> correct amount of bytes thus overwriting the end of the block. It only
> works by accident in MiNT because the allocated block is larger than you
> requested.
> 
> You should either add 1 to the return value if positive, or use
> strlen().

I *am* adding 1. In the example above, PATHSIZE = 256, and string1 and string2 
are around 20 at max. Overflow is definitely out of the question.
 
> > certainly can upload the complete sources) and, anyway, the code is 100 %
> > stable under MiNT with MP on.
> 
> No it's not ;) It only works by accident. The code is broken in all
> cases. A good catch by MagiC. If you haven't tested this under MagiC,
> your code would have caused mysterious errors even under MiNT when MP is
> disabled.

Thanks, these are very good points you're making. I'll review the code and if 
I still can't figure it out I'll post some real code and ask for help again!

Thanks again.
JFL
-- 
Jean-François Lemaire