Message-ID: <4869CCE9.1020304@sgi.com>
Date: Tue, 01 Jul 2008 16:21:29 +1000
From: Lachlan McIlroy <lachlan@sgi.com>
Reply-To: lachlan@sgi.com
User-Agent: Thunderbird 2.0.0.14 (X11/20080421)
MIME-Version: 1.0
To: Christoph Hellwig <hch@infradead.org>
CC: xfs-dev <xfs-dev@sgi.com>, xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH] Fix use after free when closing log/rt devices
References: <48647746.5010007@sgi.com> <20080627063219.GA25015@infradead.org>
In-Reply-To: <20080627063219.GA25015@infradead.org>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Sender: xfs-bounce@oss.sgi.com
Errors-to: xfs-bounce@oss.sgi.com
Precedence: bulk

Christoph Hellwig wrote:
> On Fri, Jun 27, 2008 at 03:14:46PM +1000, Lachlan McIlroy wrote:
>> The call to xfs_free_buftarg() will free the memory used by it's argument
>> so we need to save the bdev to pass to xfs_blkdev_put()
>>
>> Lachlan
>>
>> --- fs/xfs/linux-2.6/xfs_super.c_1.432	2008-06-27 14:51:17.000000000 +1000
>> +++ fs/xfs/linux-2.6/xfs_super.c	2008-06-27 14:59:26.000000000 +1000
>> @@ -781,13 +781,17 @@ STATIC void
>> xfs_close_devices(
>> 	struct xfs_mount	*mp)
>> {
>> +	struct block_device	*bdev;
>> +
>> 	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp) {
>> +		bdev = mp->m_logdev_targp->bt_bdev;
>> 		xfs_free_buftarg(mp->m_logdev_targp);
>> -		xfs_blkdev_put(mp->m_logdev_targp->bt_bdev);
>> +		xfs_blkdev_put(bdev);
>> 	}
>> 	if (mp->m_rtdev_targp) {
>> +		bdev = mp->m_rtdev_targp->bt_bdev;
>> 		xfs_free_buftarg(mp->m_rtdev_targp);
>> -		xfs_blkdev_put(mp->m_rtdev_targp->bt_bdev);
>> +		xfs_blkdev_put(bdev);
>> 	}
> 
> Looks good, alhough two local variables inside the ifs might be cleaner:
> 
> 	if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp) {
> 		struct block_device *logdev = mp->m_logdev_targp->bt_bdev;
> 
> 		xfs_free_buftarg(mp->m_logdev_targp);
> 		xfs_blkdev_put(logdev);
> 	}
> 
> 	...
> 

Thought someone might suggest that.  I'll make the changes, thanks.