[PATCH] xfs: Fix uninitialized variable in xfs_reflink_reserve_cow_range()

[PATCH] xfs: Fix uninitialized variable in xfs_reflink_reserve_cow_range()

[Geert Uytterhoeven]

with gcc 4.1.2:

    fs/xfs/xfs_reflink.c: In function ‘xfs_reflink_reserve_cow_range’:
    fs/xfs/xfs_reflink.c:327: warning: ‘error’ may be used uninitialized in this function

Indeed, if "count" is zero, the function will return an uninitialized
error value.

While "count" is unlikely to be zero, this function is called through
the public iomap API. Hence fix this by preinitializing error to zero.

Fixes: 2a06705cd5954030 ("xfs: create delalloc extents in CoW fork")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 fs/xfs/xfs_reflink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 5965e9455d91e036..d48a7cc2fe007f66 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -324,7 +324,7 @@
 	struct xfs_mount	*mp = ip->i_mount;
 	xfs_fileoff_t		offset_fsb, end_fsb;
 	bool			skipped = false;
-	int			error;
+	int			error = 0;
 
 	trace_xfs_reflink_reserve_cow_range(ip, offset, count);
 
-- 
1.9.1

Re: [PATCH] xfs: Fix uninitialized variable in xfs_reflink_reserve_cow_range()

[Christoph Hellwig]

On Mon, Oct 17, 2016 at 12:16:44PM +0200, Geert Uytterhoeven wrote:
> with gcc 4.1.2:
> 
>     fs/xfs/xfs_reflink.c: In function ‘xfs_reflink_reserve_cow_range’:
>     fs/xfs/xfs_reflink.c:327: warning: ‘error’ may be used uninitialized in this function
> 
> Indeed, if "count" is zero, the function will return an uninitialized
> error value.
> 
> While "count" is unlikely to be zero, this function is called through
> the public iomap API. Hence fix this by preinitializing error to zero.

The iomap API should never call in with a zero count, but I think the
initialization is a fine safety net anyway:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Btw, what compiler did you get this from?  I haven't seen it, but then
again I recently missed a lot of initializers without compiler warnings,
so either something changed in the Debian stable gcc or our build
system recently..

Re: [PATCH] xfs: Fix uninitialized variable in xfs_reflink_reserve_cow_range()

[Geert Uytterhoeven]

Hi Christoph,

On Mon, Oct 17, 2016 at 2:08 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Oct 17, 2016 at 12:16:44PM +0200, Geert Uytterhoeven wrote:
>> with gcc 4.1.2:
>>
>>     fs/xfs/xfs_reflink.c: In function ‘xfs_reflink_reserve_cow_range’:
>>     fs/xfs/xfs_reflink.c:327: warning: ‘error’ may be used uninitialized in this function
>>
>> Indeed, if "count" is zero, the function will return an uninitialized
>> error value.
>>
>> While "count" is unlikely to be zero, this function is called through
>> the public iomap API. Hence fix this by preinitializing error to zero.
>
> The iomap API should never call in with a zero count, but I think the
> initialization is a fine safety net anyway:

Exactly my thought.

> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!

> Btw, what compiler did you get this from?  I haven't seen it, but then
> again I recently missed a lot of initializers without compiler warnings,
> so either something changed in the Debian stable gcc or our build
> system recently..

m68k-linux-gnu-gcc version 4.1.2 20061115 (prerelease) (Ubuntu 4.1.1-21)

Stoneage, but it did find 4 real bugs introduced in v4.9-rc1...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] xfs: Fix uninitialized variable in xfs_reflink_reserve_cow_range()

[Darrick J. Wong]

On Mon, Oct 17, 2016 at 12:16:44PM +0200, Geert Uytterhoeven wrote:
> with gcc 4.1.2:
> 
>     fs/xfs/xfs_reflink.c: In function ‘xfs_reflink_reserve_cow_range’:
>     fs/xfs/xfs_reflink.c:327: warning: ‘error’ may be used uninitialized in this function
> 
> Indeed, if "count" is zero, the function will return an uninitialized
> error value.
> 
> While "count" is unlikely to be zero, this function is called through
> the public iomap API. Hence fix this by preinitializing error to zero.
> 
> Fixes: 2a06705cd5954030 ("xfs: create delalloc extents in CoW fork")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/xfs_reflink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 5965e9455d91e036..d48a7cc2fe007f66 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -324,7 +324,7 @@
>  	struct xfs_mount	*mp = ip->i_mount;
>  	xfs_fileoff_t		offset_fsb, end_fsb;
>  	bool			skipped = false;
> -	int			error;
> +	int			error = 0;
>  
>  	trace_xfs_reflink_reserve_cow_range(ip, offset, count);
>  
> -- 
> 1.9.1
>