xfs
[Top] [All Lists]

Re: [PATCH v2] xfs: overflow in xfs_iomap_eof_align_last_fsb

To: Peter Watkins <treestem@xxxxxxxxx>
Subject: Re: [PATCH v2] xfs: overflow in xfs_iomap_eof_align_last_fsb
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Tue, 2 Dec 2014 11:24:40 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1417473928-447-1-git-send-email-treestem@xxxxxxxxx>
References: <1416855973-6359-1-git-send-email-treestem@xxxxxxxxx> <1417473928-447-1-git-send-email-treestem@xxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Mon, Dec 01, 2014 at 05:45:28PM -0500, Peter Watkins wrote:
> If extsize is set and new_last_fsb is larger than 32 bits, the
> roundup to extsize will overflow the align variable. Instead,
> combine alignments by rounding stripe size up to extsize.
> 
> Signed-off-by: Peter Watkins <treestem@xxxxxxxxx>
> Reviewed-by: Nathaniel W. Turner <nate@xxxxxxxxxxxxxxx>
> ---

Looks fine to me:

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  fs/xfs/xfs_iomap.c |   13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index afcf3c9..3fad071 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -52,7 +52,6 @@ xfs_iomap_eof_align_last_fsb(
>       xfs_extlen_t    extsize,
>       xfs_fileoff_t   *last_fsb)
>  {
> -     xfs_fileoff_t   new_last_fsb = 0;
>       xfs_extlen_t    align = 0;
>       int             eof, error;
>  
> @@ -70,8 +69,8 @@ xfs_iomap_eof_align_last_fsb(
>               else if (mp->m_dalign)
>                       align = mp->m_dalign;
>  
> -             if (align && XFS_ISIZE(ip) >= XFS_FSB_TO_B(mp, align))
> -                     new_last_fsb = roundup_64(*last_fsb, align);
> +             if (align && XFS_ISIZE(ip) < XFS_FSB_TO_B(mp, align))
> +                     align = 0;
>       }
>  
>       /*
> @@ -79,14 +78,14 @@ xfs_iomap_eof_align_last_fsb(
>        * (when file on a real-time subvolume or has di_extsize hint).
>        */
>       if (extsize) {
> -             if (new_last_fsb)
> -                     align = roundup_64(new_last_fsb, extsize);
> +             if (align)
> +                     align = roundup_64(align, extsize);
>               else
>                       align = extsize;
> -             new_last_fsb = roundup_64(*last_fsb, align);
>       }
>  
> -     if (new_last_fsb) {
> +     if (align) {
> +             xfs_fileoff_t   new_last_fsb = roundup_64(*last_fsb, align);
>               error = xfs_bmap_eof(ip, new_last_fsb, XFS_DATA_FORK, &eof);
>               if (error)
>                       return error;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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