Bug 11401
| Summary: | pktcdvd: BUG, NULL pointer dereference in pkt_ioctl, bisected | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | Laurent Riffard (laurent.riffard) |
| Component: | Other | Assignee: | drivers_other |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | axboe, bunk, rjw |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.27-rc4 | Subsystem: | |
| Regression: | Yes | Bisected commit-id: | |
| Bug Depends on: | |||
| Bug Blocks: | 11167 | ||
| Attachments: |
Full dmesg
.config |
||
Created attachment 17372 [details]
Full dmesg
Created attachment 17373 [details]
.config
Yes blkdev_ioctl is buggy and needs updating for a whole load of ioctls On Sunday, 24 of August 2008, Linus Torvalds wrote:
>
> On Sat, 23 Aug 2008, Rafael J. Wysocki wrote:
> >
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=11401
> > Subject : pktcdvd: BUG, NULL pointer dereference in pkt_ioctl,
> bisected
> > Submitter : Laurent Riffard <laurent.riffard@free.fr>
> > Date : 2008-08-22 08:16 (2 days old)
>
> This one looks irritating.
>
> It's bisected to 5b6155ee70e9c4d2ad7e6f514c8eee06e2711c3a ("pktcdvd: push
> BKL down into driver"), but the problem goes deeper than that.
>
> The "unlocked" ioctl's do not get a "struct inode *" pointer, they _only_
> get the "struct file *". And this is very much historical usage, where
> some internal functions only passed in the inode (good or not, whatever).
>
> And ioctl_by_bdev() doesn't have a "struct file *" and has depended on
> passing in a NUMM "struct file *" and its own "struct inode *", and
> expects the ioctl's to just use that instead. But the unlocked ioctl just
> drops it on the floor, and uses just the (unusable) file pointer.
>
> Grr.
>
> And some other cases (like pkt_ioctl() itself) that simply pass in a
> _different_ inode than the file itself is attached to. It does
>
> blkdev_ioctl(pd->bdev->bd_inode, file, cmd, arg);
>
> where "file" points to the pkt_ioctl thing, but "inode" points to the
> inode "behind" the pkt interface.
>
> Double grr.
>
> I really think the only sane model is to literally make "unlocked_ioctl()"
> have the same calling convention as the old "ioctl()" thing had, and pass
> in both file * and inode *. It was a stupid "cleanup" to try to have a
> simpler interface for the unlocked version. Having two different models,
> where we have actually _depended_ on the old model and then are trying to
> convert to a (weaker) new model, is not a good idea.
>
> The alternative is to do this _only_ for the blkdev_ioctl's, and have
> those only take the "inode *", and then create a new fake "struct file *"
> to go with it, regardless of what "struct file" was passed in (exactly
> because the blockdev ones really think that the inode is the important
> part).
>
> Hmm?
>
> We need to fix this.
fixed by commit 8560c650f340565b720fd57d1f9c99ab216d99d0 |
Latest working kernel version: 2.6.26 Earliest failing kernel version: 2.6.27-rc1 Steps to reproduce: # pktsetup dvd /dev/dvdrw1 pktcdvd: writer pktcdvd0 mapped to hdc (insert an UDF-formatted DVD-RW) # mount -tauto /dev/pktcdvd/dvd /media/pkt pktcdvd: Fixed packets, 16 blocks, Mode-1 disc pktcdvd: write speed 2770kB/s pktcdvd: 4595774kB available on disc BUG: unable to handle kernel NULL pointer dereference at 0000000c IP: [<e1cd15be>] :pktcdvd:pkt_ioctl+0xa/0x97 *pde = 00000000 Oops: 0000 [#1] PREEMPT Modules linked in: udf pktcdvd radeon drm lp nls_iso8859_1 nls_cp850 vfat fat reiserfs ipv6 eeprom w83781d hwmon_vid usblp snd_ens1371 gameport snd_ac97_codec af_packet ac97_bus snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_oss snd_seq_midi snd_rawmidi sr_mod snd_seq_midi_event cdrom sg snd_seq snd_timer firewire_ohci snd_seq_device firewire_core snd crc_itu_t via686a ata_generic parport_pc soundcore uhci_hcd via_agp snd_page_alloc i2c_viapro parport agpgart ohci1394 usbcore ieee1394 evdev rtc pcspkr floppy dm_snapshot reiser4 lzo_decompress lzo_compress sd_mod pata_via libata scsi_mod dock dm_mirror dm_log dm_mod Pid: 4310, comm: mount Not tainted (2.6.27-rc4-00022-g7a10e7d #65) EIP: 0060:[<e1cd15be>] EFLAGS: 00010286 CPU: 0 EIP is at pkt_ioctl+0xa/0x97 [pktcdvd] EAX: 00000000 EBX: e1cd15b4 ECX: de910dcc EDX: 00005310 ESI: 00005310 EDI: 00000000 EBP: de910dcc ESP: de910c90 DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068 Process mount (pid: 4310, ti=de910000 task=dfbeadc0 task.ti=de910000) Stack: e1cd15b4 dfaae000 00000000 de910dcc c01bd38a dba2cdb4 00005310 fffffdfd 00000000 dba2cd00 c01bdaeb 00005310 de910dcc dba2cd0c c028e865 dba2cd00 00000000 00005310 00000000 dba2cdb4 dfaae000 dba2cd0c e1cd7000 00000000 Call Trace: [<e1cd15b4>] pkt_ioctl+0x0/0x97 [pktcdvd] [<c01bd38a>] blkdev_driver_ioctl+0x26/0x5b [<c01bdaeb>] blkdev_ioctl+0x72c/0x74d [<c028e865>] __mutex_unlock_slowpath+0xdf/0xf1 [<c017b786>] __blkdev_get+0x63/0x6e [<c01c70ed>] string+0x27/0x6d [<c01c73ae>] vsnprintf+0x27b/0x43e [<c01588b6>] cpu_cache_get+0x8/0xd [<c01593e5>] ____cache_alloc+0x14/0x469 [<c017bb10>] ioctl_by_bdev+0x23/0x32 [<e1e1b6f8>] udf_get_last_session+0x1a/0x32 [udf] [<e1e1f97e>] udf_fill_super+0x162/0x7a3 [udf] [<c01588b6>] cpu_cache_get+0x8/0xd [<c018b918>] disk_name+0x1f/0x5b [<c01cffff>] pci_read_config+0x9d/0x1a6 [<c015e0ce>] get_sb_bdev+0xc9/0x10b [<e1e1dffb>] udf_get_sb+0x12/0x16 [udf] [<e1e1f81c>] udf_fill_super+0x0/0x7a3 [udf] [<c015db6c>] vfs_kern_mount+0x39/0x72 [<c015de71>] do_kern_mount+0x2f/0xad [<c016e1ed>] do_new_mount+0x55/0x89 [<c016f143>] do_mount+0x17c/0x19b [<c0145628>] alloc_pages_node+0x1a/0x1d [<c014565f>] __get_free_pages+0xb/0x26 [<c016da86>] copy_mount_options+0x26/0x10d [<c016f5f0>] sys_mount+0x6d/0xaa [<c01038cd>] sysenter_do_call+0x12/0x35 ======================= Code: a6 78 cd ff c6 44 24 04 1e 85 f6 58 89 f8 89 e2 0f 95 44 24 04 e8 3a fd ff ff 83 c4 2c 5b 5e 5f c3 55 89 cd 57 89 c7 56 89 d6 53 <8b> 40 0c 8b 58 24 e8 6b e3 5b de 81 fe 09 53 00 00 8b 83 b4 01 EIP: [<e1cd15be>] pkt_ioctl+0xa/0x97 [pktcdvd] SS:ESP 0068:de910c90 ---[ end trace 0986c4dfec814e67 ]--- git-bisect said: 5b6155ee70e9c4d2ad7e6f514c8eee06e2711c3a is first bad commit commit 5b6155ee70e9c4d2ad7e6f514c8eee06e2711c3a Author: Alan Cox <alan@lxorguk.ukuu.org.uk> Date: Fri Jul 4 09:29:16 2008 +0200 pktcdvd: push BKL down into driver Push the lock_kernel down into the driver and switch to unlocked_ioctl [akpm@linux-foundation.org: build fix] Signed-off-by: Alan Cox <alan@redhat.com> Acked-by: Peter Osterlund <petero2@telia.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Jens Axboe <jens.axboe@oracle.com> :040000 040000 0c7aeb48357bd31ac8f115f89106f5182e25c3ee b8363c3541c4d00a83ab6b28657dabcef890a7dc M drivers Indeed, udf_get_last_session calls ioctl_by_bdev, which calls blkdev_ioctl with a NULL file. As a result, pkt_ioctl is called with a NULL file.