[bug report] md: range check slot number when manually adding a spare.

[Dan Carpenter <error27@xxxxxxxxx>]

[ Ancient code, but you're still at the same email address...  -dan ]

Hello NeilBrown,

The patch ba1b41b6b4e3: "md: range check slot number when manually
adding a spare." from Jan 14, 2011, leads to the following Smatch
static checker warning:

drivers/md/md.c:3170 slot_store() warn: no lower bound on 'slot'
drivers/md/md.c:3172 slot_store() warn: no lower bound on 'slot'
drivers/md/md.c:3190 slot_store() warn: no lower bound on 'slot'

drivers/md/md.c
    3117 static ssize_t
    3118 slot_store(struct md_rdev *rdev, const char *buf, size_t len)
    3119 {
    3120         int slot;
    3121         int err;
    3122 
    3123         if (test_bit(Journal, &rdev->flags))
    3124                 return -EBUSY;
    3125         if (strncmp(buf, "none", 4)==0)
    3126                 slot = -1;
    3127         else {
    3128                 err = kstrtouint(buf, 10, (unsigned int *)&slot);

slot comes from the user.

    3129                 if (err < 0)
    3130                         return err;
    3131         }
    3132         if (rdev->mddev->pers && slot == -1) {
    3133                 /* Setting 'slot' on an active array requires also
    3134                  * updating the 'rd%d' link, and communicating
    3135                  * with the personality with ->hot_*_disk.
    3136                  * For now we only support removing
    3137                  * failed/spare devices.  This normally happens automatically,
    3138                  * but not when the metadata is externally managed.
    3139                  */
    3140                 if (rdev->raid_disk == -1)
    3141                         return -EEXIST;
    3142                 /* personality does all needed checks */
    3143                 if (rdev->mddev->pers->hot_remove_disk == NULL)
    3144                         return -EINVAL;
    3145                 clear_bit(Blocked, &rdev->flags);
    3146                 remove_and_add_spares(rdev->mddev, rdev);
    3147                 if (rdev->raid_disk >= 0)
    3148                         return -EBUSY;
    3149                 set_bit(MD_RECOVERY_NEEDED, &rdev->mddev->recovery);
    3150                 md_wakeup_thread(rdev->mddev->thread);
    3151         } else if (rdev->mddev->pers) {
    3152                 /* Activating a spare .. or possibly reactivating
    3153                  * if we ever get bitmaps working here.
    3154                  */
    3155                 int err;
    3156 
    3157                 if (rdev->raid_disk != -1)
    3158                         return -EBUSY;
    3159 
    3160                 if (test_bit(MD_RECOVERY_RUNNING, &rdev->mddev->recovery))
    3161                         return -EBUSY;
    3162 
    3163                 if (rdev->mddev->pers->hot_add_disk == NULL)
    3164                         return -EINVAL;
    3165 
    3166                 if (slot >= rdev->mddev->raid_disks &&
    3167                     slot >= rdev->mddev->raid_disks + rdev->mddev->delta_disks)
    3168                         return -ENOSPC;

-1 is valid, but should this check if slot < -1?

    3169 
--> 3170                 rdev->raid_disk = slot;


regards,
dan carpenter