linux-ext4 - Re: general protection fault: from release_blocks_on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49065805.1080303@redhat.com>
Date:	Mon, 27 Oct 2008 19:08:37 -0500
From:	Eric Sandeen <sandeen@...hat.com>
To:	Theodore Tso <tytso@....edu>, Eric Sandeen <sandeen@...hat.com>,
	Eric Paris <eparis@...hat.com>, linux-kernel@...r.kernel.org,
	linux-ext4@...r.kernel.org
Subject: Re: general protection fault:  from release_blocks_on_commit

Theodore Tso wrote:
> On Mon, Oct 27, 2008 at 05:26:47PM -0500, Eric Sandeen wrote:
>> Ted, you probably need some slab debugging on to hit it.
> 
> I had slab debugging enabled, but haven't been able to replicate it
> yet.  I'll do some more work to try to replicate it.
> 
>> I think the problem is that jbd2_journal_commit_transaction may call
>> __jbd2_journal_drop_transaction(journal, commit_transaction) if the
>> checkpoint lists are NULL, and this frees the commit_transaction.
> 
> I think you're right.  I would probably change the patch around so
> that after calling __jbd2_jurnal_drop_transaction(), we set
> commit_transaction to NULL, and then adding an "if
> (commit_transaction)" to the lines in questions; that way we keep the
> commit callback outside of the j_list_lock() spinlock.

Ok, I thought about that; sounds good.  will resend.

>> Also, I'm not certain that it matters, but the loop in 
>> release_blocks_on_commit() is kfreeing list entries w/o taking
>> them off the list; I suppose maybe this is safe if the whole thing
>> is getting discarded when we're done, but just to keep things sane,
>> would this make sense
> 
> There are plenty of other loops in the kernel where we go through the
> linked list and free all of the items on the list that don't bother to
> call list_del().  That was one of the things I checked when I created
> the patch.

Ok.

>> (also, I think we need to double-check use of
>> s_md_lock; it's taken when adding things to the list, but not when
>> freeing/removing ... if it's needed, isn't it needed on both ends...):
> 
> No, because the linked list is hanging off the transaction structure.
> While the transaction is active, multiple CPU's can be adding elements
> to the linked list.  But once the transaction has been committed, we
> don't have to worry about any one else trying to modify the linked list.
> 
>       	      	    	      	       	      - Ted

ok, just double checking.

Thanks,
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html