xfs
[Top] [All Lists]

Re: [PATCH] Fix use after free when closing log/rt devices

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] Fix use after free when closing log/rt devices
From: Mark Goodwin <markgw@xxxxxxx>
Date: Fri, 27 Jun 2008 16:39:39 +1000
Cc: Lachlan McIlroy <lachlan@xxxxxxx>, xfs-dev <xfs-dev@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <20080627063219.GA25015@xxxxxxxxxxxxx>
Organization: SGI Engineering
References: <48647746.5010007@xxxxxxx> <20080627063219.GA25015@xxxxxxxxxxxxx>
Reply-to: markgw@xxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.14 (Windows/20080421)


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);
        }

        ...

do we have any QA tests that test external log?

-- Mark


<Prev in Thread] Current Thread [Next in Thread>