On Sat, 12 Feb 2011, Daniel K. wrote: > Michael Tokarev wrote: > > 12.02.2011 12:34, Daniel K. wrote: > > > Jesper Juhl wrote: > > > > sprintf() is dangerous - given the wrong source string it will > > > > overflow the destination. snprintf() is safer in that at least we'll > > > > never overflow the destination. Even if overflow will never happen > > > > today, code changes over time and snprintf() is just safer in the long > > > > run. > > > > - sprintf(nm,"rd%d", rdev->raid_disk); > > > > + snprintf(nm, sizeof(nm), "rd%d", > > > > rdev->raid_disk); > > > > sysfs_remove_link(&mddev->kobj, nm); > > > What if "rd1234" get truncated to "rd123" and you remove the wrong link. > > > (No, I didn't actually bother to check how much room was allocated.) > > > > That allocation is in the line above first sprintf which you deleted. > > Sure, didn't bother, it's very difficult. > > Yeah, early morning, I cut to much, and I didn't bother to look it up again, > sorry for being lazy. Nevertheless, the actual size is of the allocation is of > no particular importance. As you've shown, the current allocation of 20 bytes > is more than enough. > > > C'mon guys, this is pointless. 20 bytes allocated for the device > > name, and this is for raid disk number. It is impossible to have > > more than 10^17 (20 bytes total, 2 for "rd" and on for the zero > > terminator) drives in a single array. > > Agreed, and this was sort of the point. > > In all probability it would not overflow, and if it did, it would be better > for it to crash and burn, than to unlink the wrong files. > Point taken. Ignore the patch. -- Jesper Juhl <jj@xxxxxxxxxxxxx> http://www.chaosbits.net/ Plain text mails only, please. Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
