Fix stray --cluster-confrim crash
PATCH 1/1 Fix stray --cluster-confrim crash
Hi Neil,
This fix is againt the md/for-next.
A --cluster-confirm without an --add (by another node) can crash the kernel.
Fix it by guarding it using a state.
Signed-off-by Goldwyn Rodrigues <[email protected]>
---
drivers/md/md-cluster.c +14 -1
@@ -42,6 +42,10 @@ struct resync_info {__le64 hi; }; +/* md_cluster_info flags */ +#define MD_CLUSTER_WAITING_FOR_NEWDISK 1 + +struct md_cluster_info {/* dlm lock space and resources for clustered raid. */ dlm_lockspace_t *lockspace;
@@ -61,6 +65,7 @@ struct md_cluster_info {struct dlm_lock_resource *no_new_dev_lockres;struct md_thread *recv_thread;struct completion newdisk_completion; +unsigned long state; };enum msg_type {
@@ -380,9 +385,11 @@ static void process_add_new_disk(struct mddev *mddev, struct cluster_msg *cmsg)snprintf (raid_slot,16 ,"RAID_DISK=%d" , cmsg->raid_slot);pr_info ("%s:%d Sending kobject change with %s and %s\n" , __func__, __LINE__, disk_uuid, raid_slot);init_completion (&cinfo->newdisk_completion); +set_bit (MD_CLUSTER_WAITING_FOR_NEWDISK , &cinfo->state);kobject_uevent_env (&disk_to_dev (mddev->gendisk)->kobj,KOBJ_CHANGE , envp);wait_for_completion_timeout (&cinfo->newdisk_completion,NEW_DEV_TIMEOUT ); +clear_bit (MD_CLUSTER_WAITING_FOR_NEWDISK , &cinfo->state); }
@@ -832,13 +839,19 @@ static int add_new_disk_finish(struct mddev *mddev)return ret; } -static void new_disk_ack (struct mddev *mddev,bool ack) +static int new_disk_ack (struct mddev *mddev,bool ack) {struct md_cluster_info *cinfo = mddev->cluster_info; +if (!test_bit (MD_CLUSTER_WAITING_FOR_NEWDISK , &cinfo->state)) { +pr_warn ("md-cluster(%s): Spurious cluster confirmation\n" ,mdname (mddev)); +return -EINVAL ; + } +if (ack)dlm_unlock_sync (cinfo->no_new_dev_lockres);complete (&cinfo->newdisk_completion); +return 0 ; }static struct md_cluster_operations cluster_ops = {
drivers/md/md-cluster.h +1 -1
@@ -21,7 +21,7 @@ struct md_cluster_operations {int (*area_resyncing)(struct mddev *mddev,sector_t lo,sector_t hi);int (*add_new_disk_start)(struct mddev *mddev,struct md_rdev *rdev);int (*add_new_disk_finish)(struct mddev *mddev); - void (*new_disk_ack)(struct mddev *mddev, bool ack); + int (*new_disk_ack)(struct mddev *mddev, bool ack); };#endif /* _MD_CLUSTER_H */
drivers/md/md.c +6 -2
@@ -5757,7 +5755,7 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)if (mddev_is_clustered (mddev) && !(info->state & ((1 <<MD_DISK_CLUSTER_ADD ) | (1 <<MD_DISK_CANDIDATE )))) { -pr_err ("%s: Cannot add to clustered mddev. Try --cluster-add\n" , +pr_err ("%s: Cannot add to clustered mddev.\n" ,mdname (mddev));return -EINVAL ; }
@@ -5855,7 +5853,11 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)if (info->state & (1 <<MD_DISK_CANDIDATE )) {/* Through --cluster-confirm */ set_bit (Candidate, &rdev->flags); - md_cluster_ops->new_disk_ack (mddev, true); + err = md_cluster_ops->new_disk_ack (mddev, true); +if (err) { +export_rdev (rdev); +return err; + } }else if (info->state & (1 <<MD_DISK_CLUSTER_ADD )) {/* --add initiated by this node */ err = md_cluster_ops->add_new_disk_start (mddev, rdev);
Quoted 107 lines · On Mon, 2 Mar 2015 10:55:49 -0600 Goldwyn Rodrigues <[email protected]> wrote:
Hi Neil,
This fix is againt the md/for-next.
A --cluster-confirm without an --add (by another node) can crash the kernel.
Fix it by guarding it using a state.
Signed-off-by Goldwyn Rodrigues <[email protected]>---
drivers/md/md-cluster.c +14
-1@@ -42,6 +42,10 @@ struct resync_info {__le64 hi; }; +/* md_cluster_info flags */ +#define MD_CLUSTER_WAITING_FOR_NEWDISK 1 + +struct md_cluster_info {/* dlm lock space and resources for clustered raid. */ dlm_lockspace_t *lockspace;@@ -61,6 +65,7 @@ struct md_cluster_info {struct dlm_lock_resource *no_new_dev_lockres;struct md_thread *recv_thread;struct completion newdisk_completion; +unsigned long state; };enum msg_type {@@ -380,9 +385,11 @@ static void process_add_new_disk(struct mddev *mddev, struct cluster_msg *cmsg)snprintf (raid_slot,16 ,"RAID_DISK=%d" , cmsg->raid_slot);pr_info ("%s:%d Sending kobject change with %s and %s\n" , __func__, __LINE__, disk_uuid, raid_slot);init_completion (&cinfo->newdisk_completion); +set_bit (MD_CLUSTER_WAITING_FOR_NEWDISK , &cinfo->state);kobject_uevent_env (&disk_to_dev (mddev->gendisk)->kobj,KOBJ_CHANGE , envp);wait_for_completion_timeout (&cinfo->newdisk_completion,NEW_DEV_TIMEOUT ); +clear_bit (MD_CLUSTER_WAITING_FOR_NEWDISK , &cinfo->state); }@@ -832,13 +839,19 @@ static int add_new_disk_finish(struct mddev *mddev)return ret; } -static void new_disk_ack (struct mddev *mddev,bool ack) +static int new_disk_ack (struct mddev *mddev,bool ack) {struct md_cluster_info *cinfo = mddev->cluster_info; +if (!test_bit (MD_CLUSTER_WAITING_FOR_NEWDISK , &cinfo->state)) { +pr_warn ("md-cluster(%s): Spurious cluster confirmation\n" ,mdname (mddev)); +return -EINVAL ; + } +if (ack)dlm_unlock_sync (cinfo->no_new_dev_lockres);complete (&cinfo->newdisk_completion); +return 0 ; }static struct md_cluster_operations cluster_ops = {drivers/md/md-cluster.h +1
-1@@ -21,7 +21,7 @@ struct md_cluster_operations {int (*area_resyncing)(struct mddev *mddev,sector_t lo,sector_t hi);int (*add_new_disk_start)(struct mddev *mddev,struct md_rdev *rdev);int (*add_new_disk_finish)(struct mddev *mddev); - void (*new_disk_ack)(struct mddev *mddev, bool ack); + int (*new_disk_ack)(struct mddev *mddev, bool ack); };#endif /* _MD_CLUSTER_H */ drivers/md/md.c +6
-3@@ -5757,7 +5755,7 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)if (mddev_is_clustered (mddev) && !(info->state & ((1 <<MD_DISK_CLUSTER_ADD ) | (1 <<MD_DISK_CANDIDATE )))) { -pr_err ("%s: Cannot add to clustered mddev. Try --cluster-add\n" , +pr_err ("%s: Cannot add to clustered mddev.\n" ,mdname (mddev));return -EINVAL ; }@@ -5855,7 +5853,11 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)if (info->state & (1 <<MD_DISK_CANDIDATE )) {/* Through --cluster-confirm */ set_bit (Candidate, &rdev->flags); - md_cluster_ops->new_disk_ack (mddev, true); + err = md_cluster_ops->new_disk_ack (mddev, true); +if (err) { +export_rdev (rdev); +return err; + } }else if (info->state & (1 <<MD_DISK_CLUSTER_ADD )) {/* --add initiated by this node */ err = md_cluster_ops->add_new_disk_start (mddev, rdev); --
Quoted 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Makes sense.
Applied, thanks.
NeilBrown