Re: [PATCH][RFC][resend] potential NULL pointer deref in XFS on failed mount
- From: "Jesper Juhl" <jesper.juhl@xxxxxxxxx>
- Date: Thu, 16 Nov 2006 23:18:00 +0100
On 16/11/06, David Chinner <dgc@xxxxxxx> wrote:
On Thu, Nov 16, 2006 at 10:18:26PM +0100, Jesper Juhl wrote:
> (got no reply on this when I originally send it on 20061031, so resending
> now that a bit of time has passed. The patch still applies cleanly to
> Linus' git tree as of today.)
>
>
> The Coverity checker spotted a potential problem in XFS.
>
> The problem is that if, in xfs_mount(), this code triggers:
>
> ...
> if (!mp->m_logdev_targp)
> goto error0;
> ...
>
> Then we'll end up calling xfs_unmountfs_close() with a NULL
> 'mp->m_logdev_targp'.
> This in turn will result in a call to xfs_free_buftarg() with its 'btp'
> argument == NULL. xfs_free_buftarg() dereferences 'btp' leading to
> a NULL pointer dereference and crash.
Interesting that coverity found that, but failed to find the other
leaks in that function from exactly the same code and error
case.....
> I think this can happen, since the fatal call to xfs_free_buftarg()
> happens when 'm_logdev_targp != m_ddev_targp' and due to a check of
> 'm_ddev_targp' against NULL in xfs_mount() (and subsequent return if it is
> NULL) the two will never both be NULL when we hit the error0 label from
> the two lines cited above.
>
> Comments welcome (please keep me on Cc: on replies).
>
> Here's a proposed patch to fix this by testing 'btp' against NULL in
> xfs_free_buftarg().
Not the right fix - we should only be trying to free valid
buftargs, which means xfs_unmountfs_close() is the correct
place to fix this....
Ok.
e.g:
- if (mp->m_logdev_targp != mp->m_ddev_targp)
+ if (mp->m_logdev_targp && (mp->m_logdev_targp != mp->m_ddev_targp))
As to the afore-mentioned leaks, if we fail to allocate a realtime
buftarg, then we will leak a reference to both the rtdev and logdev,
and if we fail to allocate an external log buftarg we'll leak a
reference to the logdev. i.e., we fail to do one or both of:
xfs_blkdev_put(logdev);
xfs_blkdev_put(rtdev);
To remove the bdev references we may have gained earlier. Normally,
these references are released by xfs_free_buftarg(), but because we
failed to allocate the buftarg, we can't drop the references via
that method....
Thanks a lot for commenting Dave.
I don't have time to look more at this tonight or tomorrow, but I'll
try to cook up a proper fix covering all cases during the weekend.
--
Jesper Juhl <jesper.juhl@xxxxxxxxx>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Follow-Ups:
- References:
- [PATCH][RFC][resend] potential NULL pointer deref in XFS on failed mount
- From: Jesper Juhl
- Re: [PATCH][RFC][resend] potential NULL pointer deref in XFS on failed mount
- From: David Chinner
- [PATCH][RFC][resend] potential NULL pointer deref in XFS on failed mount
- Prev by Date: nifty Oill for Pennis
- Next by Date: Re: [Linux-fbdev-devel] Fwd: [Suspend-devel] resume not working on acer ferrari 4005 with radeonfb enabled
- Previous by thread: Re: [PATCH][RFC][resend] potential NULL pointer deref in XFS on failed mount
- Next by thread: Re: [PATCH][RFC][resend] potential NULL pointer deref in XFS on failed mount
- Index(es):
Relevant Pages
|
