Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161016AbWJDOSL (ORCPT ); Wed, 4 Oct 2006 10:18:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1161029AbWJDOSL (ORCPT ); Wed, 4 Oct 2006 10:18:11 -0400 Received: from mx2.suse.de ([195.135.220.15]:57732 "EHLO mx2.suse.de") by vger.kernel.org with ESMTP id S1161016AbWJDOSK (ORCPT ); Wed, 4 Oct 2006 10:18:10 -0400 Date: Wed, 04 Oct 2006 16:18:08 +0200 Message-ID: From: Takashi Iwai To: Karsten Wiese Cc: linux-kernel@vger.kernel.org, alsa-devel@lists.sourceforge.net, mingo@elte.hu Subject: Re: [PATCH] Reset file->f_op in snd_card_file_remove(). Take 2 In-Reply-To: <200610041247.19624.annabellesgarden@yahoo.de> References: <200609282228.02611.annabellesgarden@yahoo.de> <200610012029.36694.annabellesgarden@yahoo.de> <200610041247.19624.annabellesgarden@yahoo.de> User-Agent: Wanderlust/2.12.0 (Your Wildest Dreams) SEMI/1.14.6 (Maruoka) FLIM/1.14.7 (=?ISO-8859-4?Q?Sanj=F2?=) APEL/10.6 MULE XEmacs/21.5 (beta25) (eggplant) (+CVS-20060326) (i386-suse-linux) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1656 Lines: 53 At Wed, 4 Oct 2006 12:47:19 +0200, Karsten Wiese wrote: > > Am Mittwoch, 4. Oktober 2006 11:22 schrieb Takashi Iwai: > > > > It should call snd_card_free_when_closed() instead. > > > IMHO, that would just make sure that the bug happens. > Please see my annotations, starting with // in: > > void fastcall __fput(struct file *file) > { > struct dentry *dentry = file->f_dentry; > struct vfsmount *mnt = file->f_vfsmnt; > struct inode *inode = dentry->d_inode; > > might_sleep(); > > fsnotify_close(file); > /* > * The function eventpoll_release() should be the first called > * in the file cleanup chain. > */ > eventpoll_release(file); > locks_remove_flock(file); > > if (file->f_op && file->f_op->release) > file->f_op->release(inode, file); > // Here snd_hwdep_release() is called. > // snd_hwdep_release() calls snd_card_file_remove(). > // snd_card_file_remove() sees card->free_on_last_close ist set, > // calls snd_card_do_free(). > // snd_card_do_free frees file->f_op but doesn't set it NULL. > // > security_file_free(file); > if (unlikely(inode->i_cdev != NULL)) > cdev_put(inode->i_cdev); > fops_put(file->f_op); > // file->f_op has already been freeed! > // fops_put(file->f_op) is likely to oops. Yes, this looks like an invalid access. The problem is that we use kmalloc for allocating a dummy f_op. IMO, the simlest solution is to use a static dummy f_op. Takashi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/