Re: linux-image-2.6.30-1-686: unable to unmount a loop device

[Ben Hutchings]

On Fri, 2009-06-26 at 13:50 +0200, Jakub Wilk wrote:
> Package: linux-image-2.6.30-1-686
> Version: 2.6.30-1
> Severity: normal
> 
> # lsmod | grep -c loop
> 0
> 
> # modprobe loop max_part=8
> 
> # dd if=/dev/zero of=/tmp/fs bs=1M count=1
> 1+0 records in
> 1+0 records out
> 1048576 bytes (1.0 MB) copied, 0.00407344 s, 257 MB/s
> 
> # mke2fs -F -q /tmp/fs
> 
> # mkdir /tmp/mnt/
> 
> # mount -o loop /tmp/fs /tmp/mnt/
> 
> # umount /tmp/mnt/
> [  284.509864] BUG: unable to handle kernel NULL pointer dereference at 00000060
> [  284.509902] IP: [<c01f6f27>] blkdev_ioctl+0x25/0x842
> [  284.509929] *pde = 00000000
> [  284.509944] Oops: 0000 [#1] SMP
> [  284.509963] last sysfs file: /sys/devices/virtual/block/loop0/removable
> [  284.509980] Modules linked in: loop ext2 tun kvm_amd kvm binfmt_misc nf_conntrack_ipv6 ip6table_filter ip6_tables nvidiafb fb_ddc vgastate xt_MARK iptable_mangle iptable_nat nf_nat ipt_REJECT xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack iptable_filter ip_tables x_tables fuse snd_hda_codec_realtek tvaudio tda7432 tuner_simple tuner_types tuner arc4 ecb snd_hda_intel snd_hda_codec bttv ir_common snd_hwdep i2c_algo_bit v4l2_common videodev v4l1_compat snd_pcm videobuf_dma_sg snd_seq snd_timer snd_seq_device rt61pci crc_itu_t rt2x00pci snd videobuf_core btcx_risc rt2x00lib soundcore snd_page_alloc led_class input_polldev mac80211 tveeprom cfg80211 eeprom_93cx6 i2c_nforce2 i2c_core evdev processor button k8temp serio_raw psmouse ext3 jbd mbcache ide_gd_mod ide_cd_mod cdrom ata_generic libata scsi_mod ide_pci_generic amd74xx forcedeth ide_core ohci_hcd ehci_hcd usbcore floppy thermal fan thermal_sys [last unloaded: loop]
> [  284.510570]
> [  284.510581] Pid: 3328, comm: umount Not tainted (2.6.30-1-686 #1) M61SME-S2
> [  284.510600] EIP: 0060:[<c01f6f27>] EFLAGS: 00010287 CPU: 1
> [  284.510617] EIP is at blkdev_ioctl+0x25/0x842

This matches the source line:
	struct gendisk *disk = bdev->bd_disk;

> [  284.510630] EAX: 00000000 EBX: 0000125f ECX: 0000125f EDX: 00000000

and bdev == NULL.

> [  284.510645] ESI: 00000000 EDI: 00000000 EBP: 00000000 ESP: c3fefe08
> [  284.510662]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> [  284.510677] Process umount (pid: 3328, ti=c3fee000 task=c3d2e750 task.ti=c3fee000)
> [  284.510694] Stack:
> [  284.510704]  c011dda9 c04ca6ec c3d2e750 c3cacc80 00000000 c3cacc80 c3cacc80 c04951c0
> [  284.510756]  c031ce8e c38630c0 00000b00 c04951c0 00000000 00000212 00000000 c3d2e904
> [  284.510814]  00000001 00000246 3e1b71e2 00000042 c040c840 0000000c c017160e 0000000c
> [  284.510876] Call Trace:
> [  284.510886]  [<c011dda9>] ? pick_next_task_fair+0x80/0x87
> [  284.510909]  [<c031ce8e>] ? __schedule+0x719/0x746
> [  284.510931]  [<c017160e>] ? release_pages+0x11c/0x124
> [  284.510953]  [<c011cc82>] ? update_curr+0x58/0x178
> [  284.510973]  [<c031cec0>] ? schedule+0x5/0x13
> [  284.510991]  [<c031cfeb>] ? schedule_timeout+0x14/0xbd
> [  284.511011]  [<c011cf9e>] ? check_preempt_wakeup+0x139/0x173
> [  284.511031]  [<c031c6c7>] ? wait_for_common+0xc1/0x112
> [  284.511051]  [<c0121ee3>] ? default_wake_function+0x0/0x8
> [  284.511075]  [<c01a9eac>] ? ioctl_by_bdev+0x20/0x2f
> [  284.511096]  [<f8463f90>] ? loop_clr_fd+0x186/0x1a2 [loop]
> [  284.511119]  [<f8463fdb>] ? lo_release+0x2f/0x53 [loop]
[...]

Since this change, lo_release() calls loop_clr_fd() with bdev = NULL:

commit bb21488482bd36eae6b30b014d93619063773fd4
Author: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Date:   Sun Mar 2 09:29:48 2008 -0500

    [PATCH] switch loop
    
    ioctl doesn't need BKL here
    
    Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>

Most actions in loop_clr_fd() that use bdev were made conditional on
bdev != NULL, with the exception of:

	if (max_part > 0)
		ioctl_by_bdev(bdev, BLKRRPART, 0);

So I think that this if() needs to test bdev as well.

Ben.

-- 
Ben Hutchings
It is impossible to make anything foolproof because fools are so ingenious.

Attachment: signature.asc
Description: This is a digitally signed message part