Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

padata: Put CPU offline callback in ONLINE section to allow failure

syzbot reported the following warning:

DEAD callback error for CPU1
WARNING: kernel/cpu.c:1463 at _cpu_down+0x759/0x1020 kernel/cpu.c:1463, CPU#0: syz.0.1960/14614

at commit 4ae12d8bd9a8 ("Merge tag 'kbuild-fixes-7.0-2' of git://git.kernel.org/pub/scm/linux/kernel/git/kbuild/linux")
which tglx traced to padata_cpu_dead() given it's the only
sub-CPUHP_TEARDOWN_CPU callback that returns an error.

Failure isn't allowed in hotplug states before CPUHP_TEARDOWN_CPU
so move the CPU offline callback to the ONLINE section where failure is
possible.

Fixes: 894c9ef9780c ("padata: validate cpumask without removed CPU during offline")
Reported-by: syzbot+123e1b70473ce213f3af@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/69af0a05.050a0220.310d8.002f.GAE@google.com/
Debugged-by: Thomas Gleixner <tglx@kernel.org>
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

authored by

Daniel Jordan and committed by
Herbert Xu
c8c4a297 7fc31dd8

+65 -64
-1
include/linux/cpuhotplug.h
··· 92 92 CPUHP_NET_DEV_DEAD, 93 93 CPUHP_IOMMU_IOVA_DEAD, 94 94 CPUHP_AP_ARM_CACHE_B15_RAC_DEAD, 95 - CPUHP_PADATA_DEAD, 96 95 CPUHP_AP_DTPM_CPU_DEAD, 97 96 CPUHP_RANDOM_PREPARE, 98 97 CPUHP_WORKQUEUE_PREP,
+4 -4
include/linux/padata.h
··· 149 149 /** 150 150 * struct padata_instance - The overall control structure. 151 151 * 152 - * @cpu_online_node: Linkage for CPU online callback. 153 - * @cpu_dead_node: Linkage for CPU offline callback. 152 + * @cpuhp_node: Linkage for CPU hotplug callbacks. 154 153 * @parallel_wq: The workqueue used for parallel work. 155 154 * @serial_wq: The workqueue used for serial work. 156 155 * @pslist: List of padata_shell objects attached to this instance. 157 156 * @cpumask: User supplied cpumasks for parallel and serial works. 157 + * @validate_cpumask: Internal cpumask used to validate @cpumask during hotplug. 158 158 * @kobj: padata instance kernel object. 159 159 * @lock: padata instance lock. 160 160 * @flags: padata flags. 161 161 */ 162 162 struct padata_instance { 163 - struct hlist_node cpu_online_node; 164 - struct hlist_node cpu_dead_node; 163 + struct hlist_node cpuhp_node; 165 164 struct workqueue_struct *parallel_wq; 166 165 struct workqueue_struct *serial_wq; 167 166 struct list_head pslist; 168 167 struct padata_cpumask cpumask; 168 + cpumask_var_t validate_cpumask; 169 169 struct kobject kobj; 170 170 struct mutex lock; 171 171 u8 flags;
+61 -59
kernel/padata.c
··· 535 535 } 536 536 537 537 /* Allocate and initialize the internal cpumask dependend resources. */ 538 - static struct parallel_data *padata_alloc_pd(struct padata_shell *ps) 538 + static struct parallel_data *padata_alloc_pd(struct padata_shell *ps, 539 + int offlining_cpu) 539 540 { 540 541 struct padata_instance *pinst = ps->pinst; 541 542 struct parallel_data *pd; ··· 562 561 563 562 cpumask_and(pd->cpumask.pcpu, pinst->cpumask.pcpu, cpu_online_mask); 564 563 cpumask_and(pd->cpumask.cbcpu, pinst->cpumask.cbcpu, cpu_online_mask); 564 + if (offlining_cpu >= 0) { 565 + __cpumask_clear_cpu(offlining_cpu, pd->cpumask.pcpu); 566 + __cpumask_clear_cpu(offlining_cpu, pd->cpumask.cbcpu); 567 + } 565 568 566 569 padata_init_reorder_list(pd); 567 570 padata_init_squeues(pd); ··· 612 607 } 613 608 614 609 /* Replace the internal control structure with a new one. */ 615 - static int padata_replace_one(struct padata_shell *ps) 610 + static int padata_replace_one(struct padata_shell *ps, int offlining_cpu) 616 611 { 617 612 struct parallel_data *pd_new; 618 613 619 - pd_new = padata_alloc_pd(ps); 614 + pd_new = padata_alloc_pd(ps, offlining_cpu); 620 615 if (!pd_new) 621 616 return -ENOMEM; 622 617 ··· 626 621 return 0; 627 622 } 628 623 629 - static int padata_replace(struct padata_instance *pinst) 624 + static int padata_replace(struct padata_instance *pinst, int offlining_cpu) 630 625 { 631 626 struct padata_shell *ps; 632 627 int err = 0; ··· 634 629 pinst->flags |= PADATA_RESET; 635 630 636 631 list_for_each_entry(ps, &pinst->pslist, list) { 637 - err = padata_replace_one(ps); 632 + err = padata_replace_one(ps, offlining_cpu); 638 633 if (err) 639 634 break; 640 635 } ··· 651 646 652 647 /* If cpumask contains no active cpu, we mark the instance as invalid. */ 653 648 static bool padata_validate_cpumask(struct padata_instance *pinst, 654 - const struct cpumask *cpumask) 649 + const struct cpumask *cpumask, 650 + int offlining_cpu) 655 651 { 656 - if (!cpumask_intersects(cpumask, cpu_online_mask)) { 652 + cpumask_copy(pinst->validate_cpumask, cpu_online_mask); 653 + 654 + /* 655 + * @offlining_cpu is still in cpu_online_mask, so remove it here for 656 + * validation. Using a sub-CPUHP_TEARDOWN_CPU hotplug state where 657 + * @offlining_cpu wouldn't be in the online mask doesn't work because 658 + * padata_cpu_offline() can fail but such a state doesn't allow failure. 659 + */ 660 + if (offlining_cpu >= 0) 661 + __cpumask_clear_cpu(offlining_cpu, pinst->validate_cpumask); 662 + 663 + if (!cpumask_intersects(cpumask, pinst->validate_cpumask)) { 657 664 pinst->flags |= PADATA_INVALID; 658 665 return false; 659 666 } ··· 681 664 int valid; 682 665 int err; 683 666 684 - valid = padata_validate_cpumask(pinst, pcpumask); 667 + valid = padata_validate_cpumask(pinst, pcpumask, -1); 685 668 if (!valid) { 686 669 __padata_stop(pinst); 687 670 goto out_replace; 688 671 } 689 672 690 - valid = padata_validate_cpumask(pinst, cbcpumask); 673 + valid = padata_validate_cpumask(pinst, cbcpumask, -1); 691 674 if (!valid) 692 675 __padata_stop(pinst); 693 676 ··· 695 678 cpumask_copy(pinst->cpumask.pcpu, pcpumask); 696 679 cpumask_copy(pinst->cpumask.cbcpu, cbcpumask); 697 680 698 - err = padata_setup_cpumasks(pinst) ?: padata_replace(pinst); 681 + err = padata_setup_cpumasks(pinst) ?: padata_replace(pinst, -1); 699 682 700 683 if (valid) 701 684 __padata_start(pinst); ··· 747 730 748 731 #ifdef CONFIG_HOTPLUG_CPU 749 732 750 - static int __padata_add_cpu(struct padata_instance *pinst, int cpu) 751 - { 752 - int err = padata_replace(pinst); 753 - 754 - if (padata_validate_cpumask(pinst, pinst->cpumask.pcpu) && 755 - padata_validate_cpumask(pinst, pinst->cpumask.cbcpu)) 756 - __padata_start(pinst); 757 - 758 - return err; 759 - } 760 - 761 - static int __padata_remove_cpu(struct padata_instance *pinst, int cpu) 762 - { 763 - if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu) || 764 - !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu)) 765 - __padata_stop(pinst); 766 - 767 - return padata_replace(pinst); 768 - } 769 - 770 733 static inline int pinst_has_cpu(struct padata_instance *pinst, int cpu) 771 734 { 772 735 return cpumask_test_cpu(cpu, pinst->cpumask.pcpu) || ··· 758 761 struct padata_instance *pinst; 759 762 int ret; 760 763 761 - pinst = hlist_entry_safe(node, struct padata_instance, cpu_online_node); 764 + pinst = hlist_entry_safe(node, struct padata_instance, cpuhp_node); 762 765 if (!pinst_has_cpu(pinst, cpu)) 763 766 return 0; 764 767 765 768 mutex_lock(&pinst->lock); 766 - ret = __padata_add_cpu(pinst, cpu); 769 + 770 + ret = padata_replace(pinst, -1); 771 + 772 + if (padata_validate_cpumask(pinst, pinst->cpumask.pcpu, -1) && 773 + padata_validate_cpumask(pinst, pinst->cpumask.cbcpu, -1)) 774 + __padata_start(pinst); 775 + 767 776 mutex_unlock(&pinst->lock); 768 777 return ret; 769 778 } 770 779 771 - static int padata_cpu_dead(unsigned int cpu, struct hlist_node *node) 780 + static int padata_cpu_offline(unsigned int cpu, struct hlist_node *node) 772 781 { 773 782 struct padata_instance *pinst; 774 783 int ret; 775 784 776 - pinst = hlist_entry_safe(node, struct padata_instance, cpu_dead_node); 785 + pinst = hlist_entry_safe(node, struct padata_instance, cpuhp_node); 777 786 if (!pinst_has_cpu(pinst, cpu)) 778 787 return 0; 779 788 780 789 mutex_lock(&pinst->lock); 781 - ret = __padata_remove_cpu(pinst, cpu); 790 + 791 + if (!padata_validate_cpumask(pinst, pinst->cpumask.pcpu, cpu) || 792 + !padata_validate_cpumask(pinst, pinst->cpumask.cbcpu, cpu)) 793 + __padata_stop(pinst); 794 + 795 + ret = padata_replace(pinst, cpu); 796 + 782 797 mutex_unlock(&pinst->lock); 783 798 return ret; 784 799 } ··· 801 792 static void __padata_free(struct padata_instance *pinst) 802 793 { 803 794 #ifdef CONFIG_HOTPLUG_CPU 804 - cpuhp_state_remove_instance_nocalls(CPUHP_PADATA_DEAD, 805 - &pinst->cpu_dead_node); 806 - cpuhp_state_remove_instance_nocalls(hp_online, &pinst->cpu_online_node); 795 + cpuhp_state_remove_instance_nocalls(hp_online, &pinst->cpuhp_node); 807 796 #endif 808 797 809 798 WARN_ON(!list_empty(&pinst->pslist)); 810 799 811 800 free_cpumask_var(pinst->cpumask.pcpu); 812 801 free_cpumask_var(pinst->cpumask.cbcpu); 802 + free_cpumask_var(pinst->validate_cpumask); 813 803 destroy_workqueue(pinst->serial_wq); 814 804 destroy_workqueue(pinst->parallel_wq); 815 805 kfree(pinst); ··· 969 961 970 962 if (!alloc_cpumask_var(&pinst->cpumask.pcpu, GFP_KERNEL)) 971 963 goto err_free_serial_wq; 972 - if (!alloc_cpumask_var(&pinst->cpumask.cbcpu, GFP_KERNEL)) { 973 - free_cpumask_var(pinst->cpumask.pcpu); 974 - goto err_free_serial_wq; 975 - } 964 + if (!alloc_cpumask_var(&pinst->cpumask.cbcpu, GFP_KERNEL)) 965 + goto err_free_p_mask; 966 + if (!alloc_cpumask_var(&pinst->validate_cpumask, GFP_KERNEL)) 967 + goto err_free_cb_mask; 976 968 977 969 INIT_LIST_HEAD(&pinst->pslist); 978 970 ··· 980 972 cpumask_copy(pinst->cpumask.cbcpu, cpu_possible_mask); 981 973 982 974 if (padata_setup_cpumasks(pinst)) 983 - goto err_free_masks; 975 + goto err_free_v_mask; 984 976 985 977 __padata_start(pinst); 986 978 ··· 989 981 990 982 #ifdef CONFIG_HOTPLUG_CPU 991 983 cpuhp_state_add_instance_nocalls_cpuslocked(hp_online, 992 - &pinst->cpu_online_node); 993 - cpuhp_state_add_instance_nocalls_cpuslocked(CPUHP_PADATA_DEAD, 994 - &pinst->cpu_dead_node); 984 + &pinst->cpuhp_node); 995 985 #endif 996 986 997 987 cpus_read_unlock(); 998 988 999 989 return pinst; 1000 990 1001 - err_free_masks: 1002 - free_cpumask_var(pinst->cpumask.pcpu); 991 + err_free_v_mask: 992 + free_cpumask_var(pinst->validate_cpumask); 993 + err_free_cb_mask: 1003 994 free_cpumask_var(pinst->cpumask.cbcpu); 995 + err_free_p_mask: 996 + free_cpumask_var(pinst->cpumask.pcpu); 1004 997 err_free_serial_wq: 1005 998 destroy_workqueue(pinst->serial_wq); 1006 999 err_put_cpus: ··· 1044 1035 ps->pinst = pinst; 1045 1036 1046 1037 cpus_read_lock(); 1047 - pd = padata_alloc_pd(ps); 1038 + pd = padata_alloc_pd(ps, -1); 1048 1039 cpus_read_unlock(); 1049 1040 1050 1041 if (!pd) ··· 1093 1084 int ret; 1094 1085 1095 1086 ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, "padata:online", 1096 - padata_cpu_online, NULL); 1087 + padata_cpu_online, padata_cpu_offline); 1097 1088 if (ret < 0) 1098 1089 goto err; 1099 1090 hp_online = ret; 1100 - 1101 - ret = cpuhp_setup_state_multi(CPUHP_PADATA_DEAD, "padata:dead", 1102 - NULL, padata_cpu_dead); 1103 - if (ret < 0) 1104 - goto remove_online_state; 1105 1091 #endif 1106 1092 1107 1093 possible_cpus = num_possible_cpus(); 1108 1094 padata_works = kmalloc_objs(struct padata_work, possible_cpus); 1109 1095 if (!padata_works) 1110 - goto remove_dead_state; 1096 + goto remove_online_state; 1111 1097 1112 1098 for (i = 0; i < possible_cpus; ++i) 1113 1099 list_add(&padata_works[i].pw_list, &padata_free_works); 1114 1100 1115 1101 return; 1116 1102 1117 - remove_dead_state: 1118 - #ifdef CONFIG_HOTPLUG_CPU 1119 - cpuhp_remove_multi_state(CPUHP_PADATA_DEAD); 1120 1103 remove_online_state: 1104 + #ifdef CONFIG_HOTPLUG_CPU 1121 1105 cpuhp_remove_multi_state(hp_online); 1122 1106 err: 1123 1107 #endif