* [PATCH 0/4] Fix error paths on device initialization in the block layer @ 2013-10-14 16:10 Mikulas Patocka 2013-10-14 16:11 ` [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails Mikulas Patocka 2013-10-15 15:32 ` [PATCH 0/4] Fix error paths on device initialization in the block layer Jens Axboe 0 siblings, 2 replies; 10+ messages in thread From: Mikulas Patocka @ 2013-10-14 16:10 UTC (permalink / raw) To: Jens Axboe Cc: Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Tejun Heo, Vivek Goyal Hi This series of patches fixes some crashes in the block layer in device initialization when memory allocation fails. The crashes were discovered during stress testing - we tried to create as many logical volumes as possible. Note that there are still some places in the kernel where error handling is inadequate, for example add_disk or blk_register_region. Mikulas ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails 2013-10-14 16:10 [PATCH 0/4] Fix error paths on device initialization in the block layer Mikulas Patocka @ 2013-10-14 16:11 ` Mikulas Patocka 2013-10-14 16:12 ` [PATCH 2/4] loop: fix crash if blk_alloc_queue fails Mikulas Patocka 2013-10-15 13:42 ` [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails Tejun Heo 2013-10-15 15:32 ` [PATCH 0/4] Fix error paths on device initialization in the block layer Jens Axboe 1 sibling, 2 replies; 10+ messages in thread From: Mikulas Patocka @ 2013-10-14 16:11 UTC (permalink / raw) To: Jens Axboe Cc: Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Tejun Heo, Vivek Goyal If blkcg_init_queue fails, blk_alloc_queue_node doesn't call bdi_destroy to clean up structures allocated by the backing dev. ------------[ cut here ]------------ WARNING: at lib/debugobjects.c:260 debug_print_object+0x85/0xa0() ODEBUG: free active (active state 0) object type: percpu_counter hint: (null) Modules linked in: dm_loop dm_mod ip6table_filter ip6_tables uvesafb cfbcopyarea cfbimgblt cfbfillrect fbcon font bitblit fbcon_rotate fbcon_cw fbcon_ud fbcon_ccw softcursor fb fbdev ipt_MASQUERADE iptable_nat nf_nat_ipv4 msr nf_conntrack_ipv4 nf_defrag_ipv4 xt_state ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc tun ipv6 cpufreq_userspace cpufreq_stats cpufreq_powersave cpufreq_ondemand cpufreq_conservative spadfs fuse hid_generic usbhid hid raid0 md_mod dmi_sysfs nf_nat_ftp nf_nat nf_conntrack_ftp nf_conntrack lm85 hwmon_vid snd_usb_audio snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep snd_usbmidi_lib snd_rawmidi snd soundcore acpi_cpufreq freq_table mperf sata_svw serverworks kvm_amd ide_core ehci_pci ohci_hcd libata ehci_hcd kvm usbcore tg3 u sb_common libphy k10temp pcspkr ptp i2c_piix4 i2c_core evdev microcode hwmon rtc_cmos pps_core e100 skge floppy mii processor button unix CPU: 0 PID: 2739 Comm: lvchange Tainted: G W 3.10.15-devel #14 Hardware name: empty empty/S3992-E, BIOS 'V1.06 ' 06/09/2009 0000000000000009 ffff88023c3c1ae8 ffffffff813c8fd4 ffff88023c3c1b20 ffffffff810399eb ffff88043d35cd58 ffffffff81651940 ffff88023c3c1bf8 ffffffff82479d90 0000000000000005 ffff88023c3c1b80 ffffffff81039a67 Call Trace: [<ffffffff813c8fd4>] dump_stack+0x19/0x1b [<ffffffff810399eb>] warn_slowpath_common+0x6b/0xa0 [<ffffffff81039a67>] warn_slowpath_fmt+0x47/0x50 [<ffffffff8122aaaf>] ? debug_check_no_obj_freed+0xcf/0x250 [<ffffffff81229a15>] debug_print_object+0x85/0xa0 [<ffffffff8122abe3>] debug_check_no_obj_freed+0x203/0x250 [<ffffffff8113c4ac>] kmem_cache_free+0x20c/0x3a0 [<ffffffff811f6709>] blk_alloc_queue_node+0x2a9/0x2c0 [<ffffffff811f672e>] blk_alloc_queue+0xe/0x10 [<ffffffffa04c0093>] dm_create+0x1a3/0x530 [dm_mod] [<ffffffffa04c6bb0>] ? list_version_get_info+0xe0/0xe0 [dm_mod] [<ffffffffa04c6c07>] dev_create+0x57/0x2b0 [dm_mod] [<ffffffffa04c6bb0>] ? list_version_get_info+0xe0/0xe0 [dm_mod] [<ffffffffa04c6bb0>] ? list_version_get_info+0xe0/0xe0 [dm_mod] [<ffffffffa04c6528>] ctl_ioctl+0x268/0x500 [dm_mod] [<ffffffff81097662>] ? get_lock_stats+0x22/0x70 [<ffffffffa04c67ce>] dm_ctl_ioctl+0xe/0x20 [dm_mod] [<ffffffff81161aad>] do_vfs_ioctl+0x2ed/0x520 [<ffffffff8116cfc7>] ? fget_light+0x377/0x4e0 [<ffffffff81161d2b>] SyS_ioctl+0x4b/0x90 [<ffffffff813cff16>] system_call_fastpath+0x1a/0x1f ---[ end trace 4b5ff0d55673d986 ]--- ------------[ cut here ]------------ This fix should be backported to stable kernels starting with 2.6.37. Note that in the kernels prior to 3.5 the affected code is different, but the bug is still there - bdi_init is called and bdi_destroy isn't. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@kernel.org # 2.6.37+ --- block/blk-core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Index: linux-3.10.15-devel/block/blk-core.c =================================================================== --- linux-3.10.15-devel.orig/block/blk-core.c 2013-10-11 18:39:03.000000000 +0200 +++ linux-3.10.15-devel/block/blk-core.c 2013-10-11 18:39:36.000000000 +0200 @@ -645,10 +645,12 @@ struct request_queue *blk_alloc_queue_no __set_bit(QUEUE_FLAG_BYPASS, &q->queue_flags); if (blkcg_init_queue(q)) - goto fail_id; + goto fail_bdi; return q; +fail_bdi: + bdi_destroy(&q->backing_dev_info); fail_id: ida_simple_remove(&blk_queue_ida, q->id); fail_q: ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] loop: fix crash if blk_alloc_queue fails 2013-10-14 16:11 ` [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails Mikulas Patocka @ 2013-10-14 16:12 ` Mikulas Patocka 2013-10-14 16:13 ` [PATCH 3/4] block: fix a probe argument to blk_register_region Mikulas Patocka 2013-10-15 13:43 ` [PATCH 2/4] loop: fix crash if blk_alloc_queue fails Tejun Heo 2013-10-15 13:42 ` [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails Tejun Heo 1 sibling, 2 replies; 10+ messages in thread From: Mikulas Patocka @ 2013-10-14 16:12 UTC (permalink / raw) To: Jens Axboe Cc: Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Tejun Heo, Vivek Goyal loop: fix crash if blk_alloc_queue fails If blk_alloc_queue fails, loop_add cleans up, but it doesn't clean up the identifier allocated with idr_alloc. That causes crash on module unload in idr_for_each(&loop_index_idr, &loop_exit_cb, NULL); where we attempt to remove non-existed device with that id. BUG: unable to handle kernel NULL pointer dereference at 0000000000000380 IP: [<ffffffff812057c9>] del_gendisk+0x19/0x2d0 PGD 43d399067 PUD 43d0ad067 PMD 0 Oops: 0000 [#1] PREEMPT SMP Modules linked in: loop(-) dm_snapshot dm_zero dm_mirror dm_region_hash dm_log dm_loop dm_mod ip6table_filter ip6_tables uvesafb cfbcopyarea cfbimgblt cfbfillrect fbcon font bitblit fbcon_rotate fbcon_cw fbcon_ud fbcon_ccw softcursor fb fbdev msr ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_state ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc tun ipv6 cpufreq_userspace cpufreq_stats cpufreq_ondemand cpufreq_conservative cpufreq_powersave spadfs fuse hid_generic usbhid hid raid0 md_mod dmi_sysfs nf_nat_ftp nf_nat nf_conntrack_ftp nf_conntrack snd_usb_audio snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd_page_alloc lm85 hwmon_vid snd_hwdep snd_usbmidi_lib snd_rawmidi snd soundcore acpi_cpufreq ohci_hcd freq_table tg3 ehci_pci mperf ehci_hc d kvm_amd kvm sata_svw serverworks libphy libata ide_core k10temp usbcore hwmon microcode ptp pcspkr pps_core e100 skge mii usb_common i2c_piix4 floppy evdev rtc_cmos i2c_core processor but! ton unix CPU: 7 PID: 2735 Comm: rmmod Tainted: G W 3.10.15-devel #15 Hardware name: empty empty/S3992-E, BIOS 'V1.06 ' 06/09/2009 task: ffff88043d38e780 ti: ffff88043d21e000 task.ti: ffff88043d21e000 RIP: 0010:[<ffffffff812057c9>] [<ffffffff812057c9>] del_gendisk+0x19/0x2d0 RSP: 0018:ffff88043d21fe10 EFLAGS: 00010282 RAX: ffffffffa05102e0 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000000 RSI: ffff88043ea82800 RDI: 0000000000000000 RBP: ffff88043d21fe48 R08: 0000000000000000 R09: 0000000000000001 R10: 0000000000000001 R11: 0000000000000000 R12: 00000000000000ff R13: 0000000000000080 R14: 0000000000000000 R15: ffff88043ea82800 FS: 00007ff646534700(0000) GS:ffff880447000000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b CR2: 0000000000000380 CR3: 000000043e9bf000 CR4: 00000000000007e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Stack: ffffffff8100aba4 0000000000000092 ffff88043d21fe48 ffff88043ea82800 00000000000000ff ffff88043d21fe98 0000000000000000 ffff88043d21fe60 ffffffffa05102b4 0000000000000000 ffff88043d21fe70 ffffffffa05102ec Call Trace: [<ffffffff8100aba4>] ? native_sched_clock+0x24/0x80 [<ffffffffa05102b4>] loop_remove+0x14/0x40 [loop] [<ffffffffa05102ec>] loop_exit_cb+0xc/0x10 [loop] [<ffffffff81217b74>] idr_for_each+0x104/0x190 [<ffffffffa05102e0>] ? loop_remove+0x40/0x40 [loop] [<ffffffff8109adc5>] ? trace_hardirqs_on_caller+0x105/0x1d0 [<ffffffffa05135dc>] loop_exit+0x34/0xa58 [loop] [<ffffffff810a98ea>] SyS_delete_module+0x13a/0x260 [<ffffffff81221d5e>] ? trace_hardirqs_on_thunk+0x3a/0x3f [<ffffffff813cff16>] system_call_fastpath+0x1a/0x1f Code: f0 4c 8b 6d f8 c9 c3 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 41 56 41 55 4c 8d af 80 00 00 00 41 54 53 48 89 fb 48 83 ec 18 <48> 83 bf 80 03 00 00 00 74 4d e8 98 fe ff ff 31 f6 48 c7 c7 20 RIP [<ffffffff812057c9>] del_gendisk+0x19/0x2d0 RSP <ffff88043d21fe10> CR2: 0000000000000380 ---[ end trace 64ec069ec70f1309 ]--- Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@kernel.org # 3.1+ --- drivers/block/loop.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Index: linux-2.6/drivers/block/loop.c =================================================================== --- linux-2.6.orig/drivers/block/loop.c 2013-10-11 19:54:13.000000000 +0200 +++ linux-2.6/drivers/block/loop.c 2013-10-11 22:39:06.000000000 +0200 @@ -1633,7 +1633,7 @@ static int loop_add(struct loop_device * err = -ENOMEM; lo->lo_queue = blk_alloc_queue(GFP_KERNEL); if (!lo->lo_queue) - goto out_free_dev; + goto out_free_idr; disk = lo->lo_disk = alloc_disk(1 << part_shift); if (!disk) @@ -1678,6 +1678,8 @@ static int loop_add(struct loop_device * out_free_queue: blk_cleanup_queue(lo->lo_queue); +out_free_idr: + idr_remove(&loop_index_idr, i); out_free_dev: kfree(lo); out: ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] block: fix a probe argument to blk_register_region 2013-10-14 16:12 ` [PATCH 2/4] loop: fix crash if blk_alloc_queue fails Mikulas Patocka @ 2013-10-14 16:13 ` Mikulas Patocka 2013-10-14 16:14 ` [PATCH 4/4] bdi: test bdi_init failure Mikulas Patocka 2013-10-15 13:44 ` [PATCH 3/4] block: fix a probe argument to blk_register_region Tejun Heo 2013-10-15 13:43 ` [PATCH 2/4] loop: fix crash if blk_alloc_queue fails Tejun Heo 1 sibling, 2 replies; 10+ messages in thread From: Mikulas Patocka @ 2013-10-14 16:13 UTC (permalink / raw) To: Jens Axboe Cc: Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Tejun Heo, Vivek Goyal The probe function is supposed to return NULL on failure (as we can see in kobj_lookup: kobj = probe(dev, index, data); ... if (kobj) return kobj; However, in loop and brd, it returns negative error from ERR_PTR. This causes a crash if we simulate disk allocation failure and run less -f /dev/loop0 because the negative number is interpreted as a pointer: BUG: unable to handle kernel NULL pointer dereference at 00000000000002b4 IP: [<ffffffff8118b188>] __blkdev_get+0x28/0x450 PGD 23c677067 PUD 23d6d1067 PMD 0 Oops: 0000 [#1] PREEMPT SMP Modules linked in: loop hpfs nvidia(PO) ip6table_filter ip6_tables uvesafb cfbcopyarea cfbimgblt cfbfillrect fbcon font bitblit fbcon_rotate fbcon_cw fbcon_ud fbcon_ccw softcursor fb fbdev msr ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_state ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc tun ipv6 cpufreq_stats cpufreq_ondemand cpufreq_userspace cpufreq_powersave cpufreq_conservative hid_generic spadfs usbhid hid fuse raid0 snd_usb_audio snd_pcm_oss snd_mixer_oss md_mod snd_pcm snd_timer snd_page_alloc snd_hwdep snd_usbmidi_lib dmi_sysfs snd_rawmidi nf_nat_ftp nf_nat nf_conntrack_ftp nf_conntrack snd soundcore lm85 hwmon_vid ohci_hcd ehci_pci ehci_hcd serverworks sata_svw libata acpi_cpufreq freq_table mperf ide_core usbcore kvm_amd kvm tg3 i2c_piix4 libphy microcode e100 usb_common ptp skge i2c_core pcspkr k10temp evdev floppy hwmon pps_core mii rtc_cmos button processor unix [last unloaded: nvidia] CPU: 1 PID: 6831 Comm: less Tainted: P W O 3.10.15-devel #18 Hardware name: empty empty/S3992-E, BIOS 'V1.06 ' 06/09/2009 task: ffff880203cc6bc0 ti: ffff88023e47c000 task.ti: ffff88023e47c000 RIP: 0010:[<ffffffff8118b188>] [<ffffffff8118b188>] __blkdev_get+0x28/0x450 RSP: 0018:ffff88023e47dbd8 EFLAGS: 00010286 RAX: ffffffffffffff74 RBX: ffffffffffffff74 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000001 RBP: ffff88023e47dc18 R08: 0000000000000002 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: ffff88023f519658 R13: ffffffff8118c300 R14: 0000000000000000 R15: ffff88023f519640 FS: 00007f2070bf7700(0000) GS:ffff880247400000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00000000000002b4 CR3: 000000023da1d000 CR4: 00000000000007e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Stack: 0000000000000002 0000001d00000000 000000003e47dc50 ffff88023f519640 ffff88043d5bb668 ffffffff8118c300 ffff88023d683550 ffff88023e47de60 ffff88023e47dc98 ffffffff8118c10d 0000001d81605698 0000000000000292 Call Trace: [<ffffffff8118c300>] ? blkdev_get_by_dev+0x60/0x60 [<ffffffff8118c10d>] blkdev_get+0x1dd/0x370 [<ffffffff8118c300>] ? blkdev_get_by_dev+0x60/0x60 [<ffffffff813cea6c>] ? _raw_spin_unlock+0x2c/0x50 [<ffffffff8118c300>] ? blkdev_get_by_dev+0x60/0x60 [<ffffffff8118c365>] blkdev_open+0x65/0x80 [<ffffffff8114d12e>] do_dentry_open.isra.18+0x23e/0x2f0 [<ffffffff8114d214>] finish_open+0x34/0x50 [<ffffffff8115e122>] do_last.isra.62+0x2d2/0xc50 [<ffffffff8115eb58>] path_openat.isra.63+0xb8/0x4d0 [<ffffffff81115a8e>] ? might_fault+0x4e/0xa0 [<ffffffff8115f4f0>] do_filp_open+0x40/0x90 [<ffffffff813cea6c>] ? _raw_spin_unlock+0x2c/0x50 [<ffffffff8116db85>] ? __alloc_fd+0xa5/0x1f0 [<ffffffff8114e45f>] do_sys_open+0xef/0x1d0 [<ffffffff8114e559>] SyS_open+0x19/0x20 [<ffffffff813cff16>] system_call_fastpath+0x1a/0x1f Code: 44 00 00 55 48 89 e5 41 57 49 89 ff 41 56 41 89 d6 41 55 41 54 4c 8d 67 18 53 48 83 ec 18 89 75 cc e9 f2 00 00 00 0f 1f 44 00 00 <48> 8b 80 40 03 00 00 48 89 df 4c 8b 68 58 e8 d5 a4 07 00 44 89 RIP [<ffffffff8118b188>] __blkdev_get+0x28/0x450 RSP <ffff88023e47dbd8> CR2: 00000000000002b4 ---[ end trace bb7f32dbf02398dc ]--- The brd change should be backported to stable kernels starting with 2.6.25. The loop change should be backported to stable kernels starting with 2.6.22. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Cc: stable@kernel.org # 2.6.22+ --- drivers/block/brd.c | 2 +- drivers/block/loop.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Index: linux-2.6/drivers/block/brd.c =================================================================== --- linux-2.6.orig/drivers/block/brd.c 2013-10-11 16:23:51.000000000 +0200 +++ linux-2.6/drivers/block/brd.c 2013-10-11 22:39:11.000000000 +0200 @@ -545,7 +545,7 @@ static struct kobject *brd_probe(dev_t d mutex_lock(&brd_devices_mutex); brd = brd_init_one(MINOR(dev) >> part_shift); - kobj = brd ? get_disk(brd->brd_disk) : ERR_PTR(-ENOMEM); + kobj = brd ? get_disk(brd->brd_disk) : NULL; mutex_unlock(&brd_devices_mutex); *part = 0; Index: linux-2.6/drivers/block/loop.c =================================================================== --- linux-2.6.orig/drivers/block/loop.c 2013-10-11 22:39:06.000000000 +0200 +++ linux-2.6/drivers/block/loop.c 2013-10-11 22:39:11.000000000 +0200 @@ -1743,7 +1743,7 @@ static struct kobject *loop_probe(dev_t if (err < 0) err = loop_add(&lo, MINOR(dev) >> part_shift); if (err < 0) - kobj = ERR_PTR(err); + kobj = NULL; else kobj = get_disk(lo->lo_disk); mutex_unlock(&loop_index_mutex); ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] bdi: test bdi_init failure 2013-10-14 16:13 ` [PATCH 3/4] block: fix a probe argument to blk_register_region Mikulas Patocka @ 2013-10-14 16:14 ` Mikulas Patocka 2013-10-15 13:47 ` Tejun Heo 2013-10-15 13:44 ` [PATCH 3/4] block: fix a probe argument to blk_register_region Tejun Heo 1 sibling, 1 reply; 10+ messages in thread From: Mikulas Patocka @ 2013-10-14 16:14 UTC (permalink / raw) To: Jens Axboe Cc: Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Tejun Heo, Vivek Goyal There were two places where return value from bdi_init was not tested. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> --- fs/char_dev.c | 3 ++- include/linux/backing-dev.h | 4 ++-- mm/swap.c | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) Index: linux-2.6/include/linux/backing-dev.h =================================================================== --- linux-2.6.orig/include/linux/backing-dev.h 2013-10-11 19:54:12.000000000 +0200 +++ linux-2.6/include/linux/backing-dev.h 2013-10-11 22:39:14.000000000 +0200 @@ -109,7 +109,7 @@ struct backing_dev_info { #endif }; -int bdi_init(struct backing_dev_info *bdi); +int __must_check bdi_init(struct backing_dev_info *bdi); void bdi_destroy(struct backing_dev_info *bdi); __printf(3, 4) @@ -117,7 +117,7 @@ int bdi_register(struct backing_dev_info const char *fmt, ...); int bdi_register_dev(struct backing_dev_info *bdi, dev_t dev); void bdi_unregister(struct backing_dev_info *bdi); -int bdi_setup_and_register(struct backing_dev_info *, char *, unsigned int); +int __must_check bdi_setup_and_register(struct backing_dev_info *, char *, unsigned int); void bdi_start_writeback(struct backing_dev_info *bdi, long nr_pages, enum wb_reason reason); void bdi_start_background_writeback(struct backing_dev_info *bdi); Index: linux-2.6/fs/char_dev.c =================================================================== --- linux-2.6.orig/fs/char_dev.c 2013-10-11 19:54:12.000000000 +0200 +++ linux-2.6/fs/char_dev.c 2013-10-11 22:39:14.000000000 +0200 @@ -574,7 +574,8 @@ static struct kobject *base_probe(dev_t void __init chrdev_init(void) { cdev_map = kobj_map_init(base_probe, &chrdevs_lock); - bdi_init(&directly_mappable_cdev_bdi); + if (bdi_init(&directly_mappable_cdev_bdi)) + panic("Failed to init directly mappable cdev bdi"); } Index: linux-2.6/mm/swap.c =================================================================== --- linux-2.6.orig/mm/swap.c 2013-10-11 19:54:12.000000000 +0200 +++ linux-2.6/mm/swap.c 2013-10-11 22:39:14.000000000 +0200 @@ -934,7 +934,8 @@ void __init swap_setup(void) #ifdef CONFIG_SWAP int i; - bdi_init(swapper_spaces[0].backing_dev_info); + if (bdi_init(swapper_spaces[0].backing_dev_info)) + panic("Failed to init swap bdi"); for (i = 0; i < MAX_SWAPFILES; i++) { spin_lock_init(&swapper_spaces[i].tree_lock); INIT_LIST_HEAD(&swapper_spaces[i].i_mmap_nonlinear); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] bdi: test bdi_init failure 2013-10-14 16:14 ` [PATCH 4/4] bdi: test bdi_init failure Mikulas Patocka @ 2013-10-15 13:47 ` Tejun Heo 0 siblings, 0 replies; 10+ messages in thread From: Tejun Heo @ 2013-10-15 13:47 UTC (permalink / raw) To: Mikulas Patocka Cc: Jens Axboe, Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Vivek Goyal On Mon, Oct 14, 2013 at 12:14:13PM -0400, Mikulas Patocka wrote: > There were two places where return value from bdi_init was not tested. A bit more detail would be nice. Can you at least mention that this patch adds __must_check? > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Acked-by: Tejun Heo <tj@kernel.org> Thanks. -- tejun ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] block: fix a probe argument to blk_register_region 2013-10-14 16:13 ` [PATCH 3/4] block: fix a probe argument to blk_register_region Mikulas Patocka 2013-10-14 16:14 ` [PATCH 4/4] bdi: test bdi_init failure Mikulas Patocka @ 2013-10-15 13:44 ` Tejun Heo 1 sibling, 0 replies; 10+ messages in thread From: Tejun Heo @ 2013-10-15 13:44 UTC (permalink / raw) To: Mikulas Patocka Cc: Jens Axboe, Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Vivek Goyal On Mon, Oct 14, 2013 at 12:13:24PM -0400, Mikulas Patocka wrote: > The probe function is supposed to return NULL on failure (as we can see in > kobj_lookup: kobj = probe(dev, index, data); ... if (kobj) return kobj; > > However, in loop and brd, it returns negative error from ERR_PTR. > > This causes a crash if we simulate disk allocation failure and run > less -f /dev/loop0 because the negative number is interpreted as a pointer: ... > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Cc: stable@kernel.org # 2.6.22+ Acked-by: Tejun Heo <tj@kernel.org> Thanks. -- tejun ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] loop: fix crash if blk_alloc_queue fails 2013-10-14 16:12 ` [PATCH 2/4] loop: fix crash if blk_alloc_queue fails Mikulas Patocka 2013-10-14 16:13 ` [PATCH 3/4] block: fix a probe argument to blk_register_region Mikulas Patocka @ 2013-10-15 13:43 ` Tejun Heo 1 sibling, 0 replies; 10+ messages in thread From: Tejun Heo @ 2013-10-15 13:43 UTC (permalink / raw) To: Mikulas Patocka Cc: Jens Axboe, Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Vivek Goyal On Mon, Oct 14, 2013 at 12:12:24PM -0400, Mikulas Patocka wrote: > loop: fix crash if blk_alloc_queue fails > > If blk_alloc_queue fails, loop_add cleans up, but it doesn't clean up the > identifier allocated with idr_alloc. That causes crash on module unload in > idr_for_each(&loop_index_idr, &loop_exit_cb, NULL); where we attempt to > remove non-existed device with that id. ... > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Cc: stable@kernel.org # 3.1+ Acked-by: Tejun Heo <tj@kernel.org> Thanks. -- tejun ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails 2013-10-14 16:11 ` [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails Mikulas Patocka 2013-10-14 16:12 ` [PATCH 2/4] loop: fix crash if blk_alloc_queue fails Mikulas Patocka @ 2013-10-15 13:42 ` Tejun Heo 1 sibling, 0 replies; 10+ messages in thread From: Tejun Heo @ 2013-10-15 13:42 UTC (permalink / raw) To: Mikulas Patocka Cc: Jens Axboe, Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Vivek Goyal On Mon, Oct 14, 2013 at 12:11:36PM -0400, Mikulas Patocka wrote: > If blkcg_init_queue fails, blk_alloc_queue_node doesn't call bdi_destroy > to clean up structures allocated by the backing dev. > > ------------[ cut here ]------------ > WARNING: at lib/debugobjects.c:260 debug_print_object+0x85/0xa0() > ODEBUG: free active (active state 0) object type: percpu_counter hint: (null) > Modules linked in: dm_loop dm_mod ip6table_filter ip6_tables uvesafb cfbcopyarea cfbimgblt cfbfillrect fbcon font bitblit fbcon_rotate fbcon_cw fbcon_ud fbcon_ccw softcursor fb fbdev ipt_MASQUERADE iptable_nat nf_nat_ipv4 msr nf_conntrack_ipv4 nf_defrag_ipv4 xt_state ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc tun ipv6 cpufreq_userspace cpufreq_stats cpufreq_powersave cpufreq_ondemand cpufreq_conservative spadfs fuse hid_generic usbhid hid raid0 md_mod dmi_sysfs nf_nat_ftp nf_nat nf_conntrack_ftp nf_conntrack lm85 hwmon_vid snd_usb_audio snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep snd_usbmidi_lib snd_rawmidi snd soundcore acpi_cpufreq freq_table mperf sata_svw serverworks kvm_amd ide_core ehci_pci ohci_hcd libata ehci_hcd kvm usbcore tg3 usb_common libphy k10temp pcspkr ptp i2c_piix4 i2c_core evdev microcode hwmon rtc_cmos pps_core e100 skge floppy mii processor button unix > CPU: 0 PID: 2739 Comm: lvchange Tainted: G W > 3.10.15-devel #14 > Hardware name: empty empty/S3992-E, BIOS 'V1.06 ' 06/09/2009 > 0000000000000009 ffff88023c3c1ae8 ffffffff813c8fd4 ffff88023c3c1b20 > ffffffff810399eb ffff88043d35cd58 ffffffff81651940 ffff88023c3c1bf8 > ffffffff82479d90 0000000000000005 ffff88023c3c1b80 ffffffff81039a67 > Call Trace: > [<ffffffff813c8fd4>] dump_stack+0x19/0x1b > [<ffffffff810399eb>] warn_slowpath_common+0x6b/0xa0 > [<ffffffff81039a67>] warn_slowpath_fmt+0x47/0x50 > [<ffffffff8122aaaf>] ? debug_check_no_obj_freed+0xcf/0x250 > [<ffffffff81229a15>] debug_print_object+0x85/0xa0 > [<ffffffff8122abe3>] debug_check_no_obj_freed+0x203/0x250 > [<ffffffff8113c4ac>] kmem_cache_free+0x20c/0x3a0 > [<ffffffff811f6709>] blk_alloc_queue_node+0x2a9/0x2c0 > [<ffffffff811f672e>] blk_alloc_queue+0xe/0x10 > [<ffffffffa04c0093>] dm_create+0x1a3/0x530 [dm_mod] > [<ffffffffa04c6bb0>] ? list_version_get_info+0xe0/0xe0 [dm_mod] > [<ffffffffa04c6c07>] dev_create+0x57/0x2b0 [dm_mod] > [<ffffffffa04c6bb0>] ? list_version_get_info+0xe0/0xe0 [dm_mod] > [<ffffffffa04c6bb0>] ? list_version_get_info+0xe0/0xe0 [dm_mod] > [<ffffffffa04c6528>] ctl_ioctl+0x268/0x500 [dm_mod] > [<ffffffff81097662>] ? get_lock_stats+0x22/0x70 > [<ffffffffa04c67ce>] dm_ctl_ioctl+0xe/0x20 [dm_mod] > [<ffffffff81161aad>] do_vfs_ioctl+0x2ed/0x520 > [<ffffffff8116cfc7>] ? fget_light+0x377/0x4e0 > [<ffffffff81161d2b>] SyS_ioctl+0x4b/0x90 > [<ffffffff813cff16>] system_call_fastpath+0x1a/0x1f > ---[ end trace 4b5ff0d55673d986 ]--- > ------------[ cut here ]------------ > > This fix should be backported to stable kernels starting with 2.6.37. Note > that in the kernels prior to 3.5 the affected code is different, but the > bug is still there - bdi_init is called and bdi_destroy isn't. > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Cc: stable@kernel.org # 2.6.37+ Acked-by: Tejun Heo <tj@kernel.org> Thanks. -- tejun ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] Fix error paths on device initialization in the block layer 2013-10-14 16:10 [PATCH 0/4] Fix error paths on device initialization in the block layer Mikulas Patocka 2013-10-14 16:11 ` [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails Mikulas Patocka @ 2013-10-15 15:32 ` Jens Axboe 1 sibling, 0 replies; 10+ messages in thread From: Jens Axboe @ 2013-10-15 15:32 UTC (permalink / raw) To: Mikulas Patocka Cc: Nick Piggin, Peter Zijlstra, Kay Sievers, dm-devel, Alasdair G. Kergon, Ken Chen, Al Viro, Zdenek Kabelac, Tejun Heo, Vivek Goyal On Mon, Oct 14 2013, Mikulas Patocka wrote: > Hi > > This series of patches fixes some crashes in the block layer in device > initialization when memory allocation fails. > > The crashes were discovered during stress testing - we tried to create as > many logical volumes as possible. > > Note that there are still some places in the kernel where error handling > is inadequate, for example add_disk or blk_register_region. Thanks, they all look good. Applied for 3.13. -- Jens Axboe ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-10-15 15:32 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-14 16:10 [PATCH 0/4] Fix error paths on device initialization in the block layer Mikulas Patocka 2013-10-14 16:11 ` [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails Mikulas Patocka 2013-10-14 16:12 ` [PATCH 2/4] loop: fix crash if blk_alloc_queue fails Mikulas Patocka 2013-10-14 16:13 ` [PATCH 3/4] block: fix a probe argument to blk_register_region Mikulas Patocka 2013-10-14 16:14 ` [PATCH 4/4] bdi: test bdi_init failure Mikulas Patocka 2013-10-15 13:47 ` Tejun Heo 2013-10-15 13:44 ` [PATCH 3/4] block: fix a probe argument to blk_register_region Tejun Heo 2013-10-15 13:43 ` [PATCH 2/4] loop: fix crash if blk_alloc_queue fails Tejun Heo 2013-10-15 13:42 ` [PATCH 1/4] blk-core: Fix memory corruption if blkcg_init_queue fails Tejun Heo 2013-10-15 15:32 ` [PATCH 0/4] Fix error paths on device initialization in the block layer Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.