lkml.org 
[lkml]   [2011]   [Feb]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] md: Remove risk of overflow via sprintf) by using snprintf() in md_check_recovery()
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@chaosbits.net> http://www.chaosbits.net/
Plain text mails only, please.
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html



\
 
 \ /
  Last update: 2011-02-13 21:23    [W:0.047 / U:0.136 seconds]
©2003-2014 Jasper Spaans. Advertise on this site