Skip to contentSkip to conversation

xfsprogs: fix xfs_logprint EFI entry split on log buffer

PATCH In review Mark Tinguely 2 participants 2 messages Apr 9, 2014 – Apr 14, 2014 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/1 xfsprogs: fix xfs_logprint EFI entry split on log buffer

xfs_logprint does not correctly handle EFI entries that are split across two log buffers. xfs_efi_copy_format() falsely interrupts the truncated size of the split entry as being a corrupt entry.

If the first log entry has enough information, namely the number of extents in the entry and the identifier, then display this information and a warning that this entry is truncated. Otherwise, if there is not enough information in the first log buffer, then print a message that the EFI decode was not possible. These messages are similar to split inode entries.

Example of a continued entry:
Oper (336): tid: f214bdb len: 44 clientid: TRANS flags: CONTINUE
EFI: #regs: 1 num_extents: 2 id: 0xffff880804f63900
EFI free extent data skipped (CONTINUE set, no space)

---
 logprint/log_misc.c      |   31 ++++++++++++++++++++++++++-----
 logprint/log_print_all.c |    2 +-
 logprint/logprint.h      |    2 +-
 3 files changed, 28 insertions(+), 7 deletions(-)
logprint/log_misc.c +26 -5
Index: b/logprint/log_misc.c
===================================================================
@@ -477,13 +477,17 @@ xlog_print_trans_efd(xfs_caddr_t *ptr, u
 
 
 int
-xlog_print_trans_efi(xfs_caddr_t *ptr, uint src_len)
+xlog_print_trans_efi(
+	xfs_caddr_t *ptr,
+	uint src_len,
+	int continued)
 {
     xfs_efi_log_format_t *src_f, *f;
     uint		 dst_len;
     xfs_extent_t	 *ex;
     int			 i;
     int			 error = 0;
+    int			 core_size = offsetof(xfs_efi_log_format_t, efi_extents);
 
     /*
      * memmove to ensure 8-byte alignment for the long longs in
@@ -498,17 +502,29 @@ xlog_print_trans_efi(xfs_caddr_t *ptr, u
 
     /* convert to native format */
     dst_len = sizeof(xfs_efi_log_format_t) + (src_f->efi_nextents - 1) * sizeof(xfs_extent_t);
+
+    if (continued && src_len < core_size) {
+	printf(_("EFI: Not enough data to decode further\n"));
+	return 1;
+    }
+
     if ((f = (xfs_efi_log_format_t *)malloc(dst_len)) == NULL) {
 	fprintf(stderr, _("%s: xlog_print_trans_efi: malloc failed\n"), progname);
 	exit(1);
     }
-    if (xfs_efi_copy_format((char*)src_f, src_len, f)) {
+    if (xfs_efi_copy_format((char*)src_f, src_len, f, continued)) {
 	error = 1;
 	goto error;
     }
 
     printf(_("EFI:  #regs: %d    num_extents: %d  id: 0x%llx\n"),
 	   f->efi_size, f->efi_nextents, (unsigned long long)f->efi_id);
+
+    if (continued) {
+	printf(_("EFI free extent data skipped (CONTINUE set, no space)\n"));
+	goto error;
+    }
+
     ex = f->efi_extents;
     for (i=0; i < f->efi_nextents; i++) {
 	    printf("(s: 0x%llx, l: %d) ",
@@ -1034,7 +1050,8 @@ xlog_print_record(
 		    }
 		    case XFS_LI_EFI: {
 			skip = xlog_print_trans_efi(&ptr,
-					be32_to_cpu(op_head->oh_len));
+					be32_to_cpu(op_head->oh_len),
+					continued);
 			break;
 		    }
 		    case XFS_LI_EFD: {
@@ -1572,7 +1589,11 @@ xfs_inode_item_format_convert(char *src_
 }
 
 int
-xfs_efi_copy_format(char *buf, uint len, xfs_efi_log_format_t *dst_efi_fmt)
+xfs_efi_copy_format(
+	char			  *buf,
+	uint			  len,
+	struct xfs_efi_log_format *dst_efi_fmt,
+	int			  continued)
 {
         uint i;
 	uint nextents = ((xfs_efi_log_format_t *)buf)->efi_nextents;
@@ -1580,7 +1601,7 @@ xfs_efi_copy_format(char *buf, uint len,
         uint len32 = sizeof(xfs_efi_log_format_32_t) + (nextents - 1) * sizeof(xfs_extent_32_t);
         uint len64 = sizeof(xfs_efi_log_format_64_t) + (nextents - 1) * sizeof(xfs_extent_64_t);
 
-        if (len == dst_len) {
+        if (len == dst_len || continued) {
                 memcpy((char *)dst_efi_fmt, buf, len);
                 return 0;
         } else if (len == len32) {
logprint/log_print_all.c +1 -1
Index: b/logprint/log_print_all.c
===================================================================
@@ -411,7 +411,7 @@ xlog_recover_print_efi(
 	    fprintf(stderr, _("%s: xlog_recover_print_efi: malloc failed\n"), progname);
 	    exit(1);
 	}
-	if (xfs_efi_copy_format((char*)src_f, src_len, f)) {
+	if (xfs_efi_copy_format((char*)src_f, src_len, f, 0)) {
 	    free(f);
 	    return;
 	}
logprint/logprint.h +1 -1
Index: b/logprint/logprint.h
===================================================================
@@ -47,6 +47,6 @@ extern void print_stars(void);
 
 extern xfs_inode_log_format_t *
 	xfs_inode_item_format_convert(char *, uint, xfs_inode_log_format_t *);
-extern int xfs_efi_copy_format(char *, uint, xfs_efi_log_format_t *);
+extern int xfs_efi_copy_format(char *, uint, xfs_efi_log_format_t *, int);
 
 #endif	/* LOGPRINT_H */

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 1/1
#2 lore raw
Quoted 13 lines · Mark Tinguely

xfs_logprint does not correctly handle EFI entries that are split across two log buffers. xfs_efi_copy_format() falsely interrupts the truncated size of the split entry as being a corrupt entry.

If the first log entry has enough information, namely the number of extents in the entry and the identifier, then display this information and a warning that this entry is truncated. Otherwise, if there is not enough information in the first log buffer, then print a message that the EFI decode was not possible. These messages are similar to split inode entries.

Quoted 7 lines

Example of a continued entry:
Oper (336): tid: f214bdb len: 44 clientid: TRANS flags: CONTINUE
EFI: #regs: 1 num_extents: 2 id: 0xffff880804f63900
EFI free extent data skipped (CONTINUE set, no space)

Looks fine.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs