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@xxxxxxx> > 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. Linus -- To unsubscribe from this list: send the line "unsubscribe kernel-testers" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
