Re: [PATCH] md: Remove risk of overflow via sprintf) by using snprintf() in md_check_recovery()

[Bodo Eggert]

Michael Tokarev <mjt@xxxxxxxxxx> 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);

> 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.

If you argue that you might get a buffer overflow, you'll have to check
for snprintf errors, too.
-- 
Logic: The art of being wrong with confidence... 

FriÃ, Spammer: tR@xxxxxxxxxxxxxxxxxxxx S5xk@xxxxxxxxxxxxxxxxxxxx
 loqnjg@xxxxxxxxxxxxxxxxxxxxxxxx 6hs4Axaqf@xxxxxxxxxxxxxxxxxxxxxxx

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/