On Friday, September 23, 2011 14:30:09 Simon Glass wrote:
On Fri, Sep 23, 2011 at 11:15 AM, Mike Frysinger wrote:
On Friday, September 23, 2011 13:38:52 Simon Glass wrote:
--- a/net/eth.c +++ b/net/eth.c
char buf[20];
sprintf(buf, "%pM", enetaddr);
snprintf(buf, sizeof(buf), "%pM", enetaddr);a mac address will not take more than 19 bytes. unless the sprintf code is completely busted, but if that's the case, we should fix that instead since it'd be pretty fundamentally screwed.
OK (18 bytes?)
right ... i meant there's 19 available (since 20 makes NUL), and there's no way it should hit that 19 limit.
char enetvar[32];
sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
snprintf(enetvar, sizeof(enetvar), index ? "%s%daddr" : "%saddr",base_name, index);in order for this to overflow, we have to have 1000+ eth devices (maybe more? i'd have to read the code closer)
Or base_name could be longer.
true, but base_name atm is supposed to be "eth" or "usbeth". i would hope we'd be diligent about device naming conventions rather than letting someone slip in "mysuperawesomeboardeth" (although that still wouldn't overflow ;]).
char enetvar[15];
sprintf(enetvar, index ? "eth%dmacskip" : "ethmacskip", index);
snprintf(enetvar, sizeof(enetvar),index ? "eth%dmacskip" : "ethmacskip", index);in order for this to overflow, we have to have 10000+ eth devices
please look at the realistic needs rather than blanket converting to snprintf
OK, this is fair enough. There are a couple of functions like ip_to_string() which don't get passed a length. I would prefer to see this done, so we can use snprintf() or assert() since the code then becomes more robust. But there are no bugs there at present. Thoughts?
for funcs that do generally handle arbitrary data, i don't mind including len specs, but when we're using known standards, i'd prefer to stick to the smaller/simpler code. this is a thin boot loader that is meant to be fast, and there are plenty of ways to bring down the system otherwise (and i don't think these cases are generally a robustness concern).
don't know if wolfgang has an opinion. -mike