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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 





[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux