Home Original page

[m68k] Final part of architecture cleanup

Nathan Sidwell nathan@codesourcery.com
Fri Mar 24 11:01:00 GMT 2006 
Ben Elliston wrote:
> Hi Nathan

> The problem is that you detect the buffer overflow after it has
> occurred; it will likely be too late.  I'd like to see this code made
> more robust.
> 
> How about adding some assertions, at the very least (as the code only
> processes compile-time strings and not external untrusted data)?

Actually I have a follow up patch that fixes the strncpy issue correctly -- I 
found it was still lurking in m68k_ip, and I got bit by it.  I attach that patch.

The attached patch folds find_cf_chip into its only caller and uses it for both 
cf and m68k chips.  Rather than generate the message into a stack allocated 
array, and them copy it to a malloced area, I simply malloc the area first and 
construct directly into it.  Although this might mean the malloced area is 
longer than necessary, I don't really see the point of optimizing that in this 
error handling case.  I use a local macro to do the string appending in a safe 
manner, and you'll see I place a sentinel NUL char right at the end of the 
buffer to start with, so that when strncpy runs out of room, it'll be right 
there.  Then I detect if the buffer is full and replace the last few chars with 
'  ...', rather than die horribly.  (hm, a thought occurs, we could retry with a 
longer buffer -- I'd really like to do that as a separate change though.)

Would you prefer

a) Commit the arch cleanup as is, and then commit the patch I attach here?
b) post a combined patch for this change and the later change.

I would have preferred not fiddling with find_cf_chip at all in this first 
patch, but it needed changing because of how it traversed the cpus array.  I 
found its logic rather tortuous.  I suppose it would be possible to fix the 
overflow in find_cf_chip first, but right now that just seems like make-work.

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

-------------- next part --------------
A non-text attachment was scrubbed...
Name: overrun.patch
Type: text/x-patch
Size: 7029 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20060324/dd031996/attachment.bin>
More information about the Binutils mailing list