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

[Bodo Eggert <arcoristkaputtundbrauchtdiesentext.7eggert@nurfuerspam.de>]

Michael Tokarev <mjt@tls.msk.ru> 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@c.7eggert.dyndns.org S5xk@h.7eggert.dyndns.org
 loqnjg@GFhzy.7eggert.dyndns.org 6hs4Axaqf@ndlJ.7eggert.dyndns.org