On Sat, 04 Mar 2023, Dan Carpenter wrote:
> [ Ancient code, but you're still at the same email address... -dan ]
Patch sent. Thanks for the report.
NeilBrown
>
> 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
>
