Re: Ooops in Kernel 2.6.26.2

[Lachlan McIlroy <lachlan@sgi.com>]

Dave Chinner wrote:
> On Mon, Aug 11, 2008 at 05:57:34PM +1000, Lachlan McIlroy wrote:
>> The ticket allocation code got reworked in 2.6.26 and we now free
>> tickets whereas before we used to cache them so the use-after-free
>> went undetected.
>>
>> This patch should do the trick.
>>
>> --- a/fs/xfs/xfs_log.c	2008-08-11 17:47:18.000000000 +1000
>> +++ b/fs/xfs/xfs_log.c	2008-08-11 17:53:24.000000000 +1000
>> @@ -336,15 +364,12 @@ xfs_log_done(xfs_mount_t	*mp,
>> 	} else {
>> 		xlog_trace_loggrant(log, ticket, "xfs_log_done: (permanent)");
>> 		xlog_regrant_reserve_log_space(log, ticket);
>> -	}
>> -
>> -	/* If this ticket was a permanent reservation and we aren't
>> -	 * trying to release it, reset the inited flags; so next time
>> -	 * we write, a start record will be written out.
>> -	 */
>> -	if ((ticket->t_flags & XLOG_TIC_PERM_RESERV) &&
>> -	    (flags & XFS_LOG_REL_PERM_RESERV) == 0)
>> +		/* If this ticket was a permanent reservation and we aren't
>> +		 * trying to release it, reset the inited flags; so next time
>> +		 * we write, a start record will be written out.
>> +		 */
>> 		ticket->t_flags |= XLOG_TIC_INITED;
>> +	}
>>
>> 	return lsn;
>> }	/* xfs_log_done */
> 
> Looks sane, Lachlan. Good catch, though it makes me wonder how we
> didn't hit it in debug builds with memory poisoning turned on.
> Compiler optimisation, perhaps?

Memory poisoning will only trigger a panic if we use the contents
of the freed structure as an address and dereference it.  For the
code above the compiler will take the contents of 'ticket' (which
is still an address) and add the offset of t_flags and then modify
the contents at that address (ie modify the poison pattern).

I think this panic was caused by the page that contained the freed
ticket being unmapped from the kernel - that just comes down to
getting the timing right.