xfs
[Top] [All Lists]

Re: [PATCH 10/11] xfsprogs: fix possible null pointer dereference in xfs

To: Vivek Trivedi <t.vivek@xxxxxxxxxxx>
Subject: Re: [PATCH 10/11] xfsprogs: fix possible null pointer dereference in xfs_iformat_extents
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 3 Dec 2015 17:31:06 +1100
Cc: xfs@xxxxxxxxxxx, a.sahrawat@xxxxxxxxxxx, pankaj.m@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1449055167-19936-11-git-send-email-t.vivek@xxxxxxxxxxx>
References: <1449055167-19936-1-git-send-email-t.vivek@xxxxxxxxxxx> <1449055167-19936-11-git-send-email-t.vivek@xxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Dec 02, 2015 at 04:49:26PM +0530, Vivek Trivedi wrote:
> Fix possible null pointer dereference in xfs_iformat_extents and
> xfs_iext_get_ext if fail to locate inode record.
> Reported by coverity.

ignore it. Code 
> 
> Signed-off-by: Vivek Trivedi <t.vivek@xxxxxxxxxxx>
> ---
>  libxfs/xfs_inode_fork.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libxfs/xfs_inode_fork.c b/libxfs/xfs_inode_fork.c
> index e1968b4..36aa0c8 100644
> --- a/libxfs/xfs_inode_fork.c
> +++ b/libxfs/xfs_inode_fork.c
> @@ -331,6 +331,8 @@ xfs_iformat_extents(
>               xfs_validate_extents(ifp, nex, XFS_EXTFMT_INODE(ip));
>               for (i = 0; i < nex; i++, dp++) {
>                       xfs_bmbt_rec_host_t *ep = xfs_iext_get_ext(ifp, i);
> +                     if (!ep)
> +                             return -EFSCORRUPTED;
>                       ep->l0 = get_unaligned_be64(&dp->l0);
>                       ep->l1 = get_unaligned_be64(&dp->l1);

Can't possibly happen, as nex is a count of the number of entries in
the extent list and so we will never overrun the list here.

> @@ -890,6 +892,8 @@ xfs_iext_get_ext(
>               xfs_extnum_t    page_idx = idx; /* ext index in target list */
>  
>               erp = xfs_iext_idx_to_irec(ifp, &page_idx, &erp_idx, 0);
> +             if (!erp)
> +                     return NULL;

Same again - this is guaranteed to succeed because the indexes
passed into the function are bound to within range by the caller.
The ASSERT()s at the beginning of the function enforce this....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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