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.020 / U:29.208 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site