[PATCH 0/2] libxfs: two kernelspace sync-ups

[PATCH 0/2] libxfs: two kernelspace sync-ups

[Eric Sandeen]

These are 2 patches for xfsprogs which have already been sent
for kernelspace, but since they are both more or less bugfixes
I'll send them explicitly for xfsprogs, and not wait for the next
libxfs sync-up.

Thanks,
-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] libxfs: don't send null bp to xfs_trans_brelse()

[Eric Sandeen]

In this case, if bp is null, error is set, and we send
bp to xfs_trans_brelse, which will try to dereference it.

Test whether we actualy have a buffer before we try to
free it.

Same fix as was sent for kernelspace.

Coverity spotted this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/xfs_da_btree.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/libxfs/xfs_da_btree.c b/libxfs/xfs_da_btree.c
index b70454e..b731b54 100644
--- a/libxfs/xfs_da_btree.c
+++ b/libxfs/xfs_da_btree.c
@@ -2582,7 +2582,8 @@ xfs_da_get_buf(
 				    mapp, nmap, 0);
 	error = bp ? bp->b_error : XFS_ERROR(EIO);
 	if (error) {
-		xfs_trans_brelse(trans, bp);
+		if (bp)
+			xfs_trans_brelse(trans, bp);
 		goto out_free;
 	}
 
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] libxfs: don't send null bp to xfs_trans_brelse()

[Brian Foster]

On Mon, Jun 16, 2014 at 06:14:20PM -0500, Eric Sandeen wrote:
> In this case, if bp is null, error is set, and we send
> bp to xfs_trans_brelse, which will try to dereference it.
> 
> Test whether we actualy have a buffer before we try to
> free it.
> 
> Same fix as was sent for kernelspace.
> 
> Coverity spotted this.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  libxfs/xfs_da_btree.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/libxfs/xfs_da_btree.c b/libxfs/xfs_da_btree.c
> index b70454e..b731b54 100644
> --- a/libxfs/xfs_da_btree.c
> +++ b/libxfs/xfs_da_btree.c
> @@ -2582,7 +2582,8 @@ xfs_da_get_buf(
>  				    mapp, nmap, 0);
>  	error = bp ? bp->b_error : XFS_ERROR(EIO);
>  	if (error) {
> -		xfs_trans_brelse(trans, bp);
> +		if (bp)
> +			xfs_trans_brelse(trans, bp);
>  		goto out_free;
>  	}
>  
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] libxfs: fix crc field handling in xfs_sb_to/from_disk

[Eric Sandeen]

If we xfs_mdrestore an image from a non-crc filesystem, lo
and behold the restored image has gained a CRC:

# db/xfs_metadump.sh -o /dev/sdc1 - | xfs_mdrestore - test.img
# xfs_db -c "sb 0" -c "p crc" /dev/sdc1
crc = 0 (correct)
# xfs_db -c "sb 0" -c "p crc" test.img
crc = 0xb6f8d6a0 (correct)

This is because xfs_sb_from_disk doesn't fill in sb_crc,
but xfs_sb_to_disk(XFS_SB_ALL_BITS) does write the in-memory
CRC to disk - so we get uninitialized memory on disk.

Fix this by always initializing sb_crc to 0 when we read
the superblock, and masking out the CRC bit from ALL_BITS
when we write it.

This same fix has already been sent for kernelspace.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/xfs_sb.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 7ee4612..ea89367 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -408,6 +408,8 @@ xfs_sb_from_disk(
 	to->sb_features_incompat = be32_to_cpu(from->sb_features_incompat);
 	to->sb_features_log_incompat =
 				be32_to_cpu(from->sb_features_log_incompat);
+	/* crc is only used on disk, not in memory; just init to 0 here. */
+	to->sb_crc = 0;
 	to->sb_pad = 0;
 	to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
 	to->sb_lsn = be64_to_cpu(from->sb_lsn);
@@ -485,6 +487,9 @@ xfs_sb_to_disk(
 	if (!fields)
 		return;
 
+	/* We should never write the crc here, it's updated in the IO path */
+	fields &= ~XFS_SB_CRC;
+
 	xfs_sb_quota_to_disk(to, from, &fields);
 	while (fields) {
 		f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] libxfs: fix crc field handling in xfs_sb_to/from_disk

[Brian Foster]

On Mon, Jun 16, 2014 at 06:14:21PM -0500, Eric Sandeen wrote:
> If we xfs_mdrestore an image from a non-crc filesystem, lo
> and behold the restored image has gained a CRC:
> 
> # db/xfs_metadump.sh -o /dev/sdc1 - | xfs_mdrestore - test.img
> # xfs_db -c "sb 0" -c "p crc" /dev/sdc1
> crc = 0 (correct)
> # xfs_db -c "sb 0" -c "p crc" test.img
> crc = 0xb6f8d6a0 (correct)
> 
> This is because xfs_sb_from_disk doesn't fill in sb_crc,
> but xfs_sb_to_disk(XFS_SB_ALL_BITS) does write the in-memory
> CRC to disk - so we get uninitialized memory on disk.
> 
> Fix this by always initializing sb_crc to 0 when we read
> the superblock, and masking out the CRC bit from ALL_BITS
> when we write it.
> 
> This same fix has already been sent for kernelspace.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  libxfs/xfs_sb.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
> index 7ee4612..ea89367 100644
> --- a/libxfs/xfs_sb.c
> +++ b/libxfs/xfs_sb.c
> @@ -408,6 +408,8 @@ xfs_sb_from_disk(
>  	to->sb_features_incompat = be32_to_cpu(from->sb_features_incompat);
>  	to->sb_features_log_incompat =
>  				be32_to_cpu(from->sb_features_log_incompat);
> +	/* crc is only used on disk, not in memory; just init to 0 here. */
> +	to->sb_crc = 0;
>  	to->sb_pad = 0;
>  	to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
>  	to->sb_lsn = be64_to_cpu(from->sb_lsn);
> @@ -485,6 +487,9 @@ xfs_sb_to_disk(
>  	if (!fields)
>  		return;
>  
> +	/* We should never write the crc here, it's updated in the IO path */
> +	fields &= ~XFS_SB_CRC;
> +
>  	xfs_sb_quota_to_disk(to, from, &fields);
>  	while (fields) {
>  		f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs