Skip to contentSkip to conversation

xfs_db: Allow writes of corrupted data by optionally skipping write verifiers

PATCH In review Eric Sandeen 3 participants 4 messages Mar 5, 2015 – Mar 14, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs_db: Allow writes of corrupted data by optionally skipping write verifiers

Being able to write corrupt data is handy if we wish to test repair against specific types of corruptions.

Add a "-c" option to the write command which allows this.

Note that this also skips CRC updates; it's not currently possible to write invalid data with a valid CRC; CRC recalculation is intertwined with validation.

---

db/crc.c +0 -7
index ad46c3f..861086a 100644
@@ -58,13 +58,6 @@ crc_help(void)
 
 }
 
-void
-xfs_dummy_verify(
-	struct xfs_buf *bp)
-{
-	return;
-}
-
 static int
 crc_f(
 	int		argc,
db/crc.h +0 -1
index 80ecec3..9f44615 100644
@@ -20,4 +20,3 @@ struct field;
 
 extern void	crc_init(void);
 extern void	crc_struct(const field_t *fields, int argc, char **argv);
-extern void	xfs_dummy_verify(struct xfs_buf *bp);
db/io.c +7 -0
index eb3daa1..2d18d56 100644
@@ -458,6 +458,13 @@ write_cur_bbs(void)
 }
 
 void
+xfs_dummy_verify(
+	struct xfs_buf *bp)
+{
+	return;
+}
+
+void
 write_cur(void)
 {
 	int skip_crc = (iocur_top->bp->b_ops->verify_write == xfs_dummy_verify);
db/io.h +1 -0
index 71082e6..31d96b4 100644
@@ -63,6 +63,7 @@ extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
 			bbmap_t *bbmap);
 extern void     ring_add(void);
 extern void	set_iocur_type(const struct typ *t);
+extern void	xfs_dummy_verify(struct xfs_buf *bp);
 
 /*
  * returns -1 for unchecked, 0 for bad and 1 for good
db/write.c +32 -4
index a0f14f4..a01c25a 100644
@@ -38,7 +38,7 @@ static int	write_f(int argc, char **argv);
 static void     write_help(void);
 
 static const cmdinfo_t	write_cmd =
-	{ "write", NULL, write_f, 0, -1, 0, N_("[field or value]..."),
+	{ "write", NULL, write_f, 0, -1, 0, N_("[-c] [field or value]..."),
 	  N_("write value to disk"), write_help };
 
 void
@@ -79,6 +79,7 @@ write_help(void)
 "  String mode: 'write \"This_is_a_filename\" - write null terminated string.\n"
 "\n"
 " In data mode type 'write' by itself for a list of specific commands.\n\n"
+" Specifying the -c option will allow writes of invalid (corrupt) data.\n\n"
 ));
 
 }
@@ -90,6 +91,9 @@ write_f(
 {
 	pfunc_t	pf;
 	extern char *progname;
+	int c;
+	int corrupt = 0;		/* Allow write of corrupt data; skip verification */
+	const struct xfs_buf_ops *stashed_ops = NULL; 
 
 	if (x.isreadonly & LIBXFS_ISREADONLY) {
 		dbprintf(_("%s started in read only mode, writing disabled\n"),
@@ -109,12 +113,36 @@ write_f(
 		return 0;
 	}
 
-	/* move past the "write" command */
-	argc--;
-	argv++;
+	if (argc) while ((c = getopt(argc, argv, "c")) != EOF) {
+		switch (c) {
+		case 'c':
+			corrupt = 1;
+			break;
+		default:
+			dbprintf(_("bad option for write command\n"));
+			return 0;
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+
+	if (corrupt) {
+		struct xfs_buf_ops nowrite_ops;
+
+		/* Temporarily remove write verifier to write bad data */
+		stashed_ops = iocur_top->bp->b_ops;
+		nowrite_ops.verify_read = stashed_ops->verify_read;
+		nowrite_ops.verify_write = xfs_dummy_verify;
+		iocur_top->bp->b_ops = &nowrite_ops;
+		dbprintf(_("Allowing write of corrupted data\n"));
+	}
 
 	(*pf)(DB_WRITE, cur_typ->fields, argc, argv);
 
+	if (stashed_ops)
+		iocur_top->bp->b_ops = stashed_ops;
+
 	return 0;
 }
 
man/man8/xfs_db.8 +7 -1
index 0764832..46a1f39 100644
@@ -728,7 +728,7 @@ and
 bits respectively, and their string equivalent reported
 (but no modifications are made).
 .TP
-.BI "write [" "field value" "] ..."
+.BI "write [\-c] [" "field value" "] ..."
 Write a value to disk.
 Specific fields can be set in structures (struct mode),
 or a block can be set to data values (data mode),
@@ -746,6 +746,12 @@ contents of the block can be shifted or rotated left or right, or filled
 with a sequence, a constant value, or a random value. In this mode
 .B write
 with no arguments gives more information on the allowed commands.
+.RS 1.0i
+.TP 0.4i
+.B \-c
+Skip write verifiers and CRC recalculation; allows invalid data to be written
+to disk.
+.RE
 .SH TYPES
 This section gives the fields in each structure type and their meanings.
 Note that some types of block cover multiple actual structures,

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

#1 lore raw

db/crc.c

Quoted 16 lines · Eric Sandeen

Being able to write corrupt data is handy if we wish to test repair against specific types of corruptions.

Add a "-c" option to the write command which allows this.

Note that this also skips CRC updates; it's not currently possible to write invalid data with a valid CRC; CRC recalculation is intertwined with validation.

---

db/crc.c +0 -0
index ad46c3f..861086a 100644

This depends on the previous crc command which isn't merged yet. It might be worth it to include that again here so it hopefully gets picked up.

Quoted 75 lines
db/crc.c +0 -7
@@ -58,13 +58,6 @@ crc_help(void)
 
 }
 
-void
-xfs_dummy_verify(
-	struct xfs_buf *bp)
-{
-	return;
-}
-
 static int
 crc_f(
 	int		argc,
db/crc.h +0 -1
index 80ecec3..9f44615 100644
@@ -20,4 +20,3 @@ struct field;
 
 extern void	crc_init(void);
 extern void	crc_struct(const field_t *fields, int argc, char **argv);
-extern void	xfs_dummy_verify(struct xfs_buf *bp);
db/io.c +7 -0
index eb3daa1..2d18d56 100644
@@ -458,6 +458,13 @@ write_cur_bbs(void)
 }
 
 void
+xfs_dummy_verify(
+	struct xfs_buf *bp)
+{
+	return;
+}
+
+void
 write_cur(void)
 {
 	int skip_crc = (iocur_top->bp->b_ops->verify_write == xfs_dummy_verify);
db/io.h +1 -0
index 71082e6..31d96b4 100644
@@ -63,6 +63,7 @@ extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
 			bbmap_t *bbmap);
 extern void     ring_add(void);
 extern void	set_iocur_type(const struct typ *t);
+extern void	xfs_dummy_verify(struct xfs_buf *bp);
 
 /*
  * returns -1 for unchecked, 0 for bad and 1 for good
db/write.c +2 -1
index a0f14f4..a01c25a 100644
@@ -38,7 +38,7 @@ static int	write_f(int argc, char **argv);
 static void     write_help(void);
 
 static const cmdinfo_t	write_cmd =
-	{ "write", NULL, write_f, 0, -1, 0, N_("[field or value]..."),
+	{ "write", NULL, write_f, 0, -1, 0, N_("[-c] [field or value]..."),
 	  N_("write value to disk"), write_help };
 
 void
@@ -79,6 +79,7 @@ write_help(void)
 "  String mode: 'write \"This_is_a_filename\" - write null terminated string.\n"
 "\n"
 " In data mode type 'write' by itself for a list of specific commands.\n\n"
+" Specifying the -c option will allow writes of invalid (corrupt) data.\n\n"
 ));
 
 }

db/write.c write_f( · line 96

Quoted 6 lines
db/write.c +3 -0
@@ -90,6 +91,9 @@ write_f(
 {
 	pfunc_t	pf;
 	extern char *progname;
+	int c;
+	int corrupt = 0;		/* Allow write of corrupt data; skip verification */
+	const struct xfs_buf_ops *stashed_ops = NULL; 

Trailing space here.

db/write.c write_f( · line 116

Quoted 11 lines
db/write.c +1 -3
 	if (x.isreadonly & LIBXFS_ISREADONLY) {
 		dbprintf(_("%s started in read only mode, writing disabled\n"),
@@ -109,12 +113,36 @@ write_f(
 		return 0;
 	}
 
-	/* move past the "write" command */
-	argc--;
-	argv++;
+	if (argc) while ((c = getopt(argc, argv, "c")) != EOF) {

Isn't argc always going to be non-zero at this point (e.g., argv[0] == "write")?

Quoted 6 lines
db/write.c +6 -0
+		switch (c) {
+		case 'c':
+			corrupt = 1;
+			break;
+		default:
+			dbprintf(_("bad option for write command\n"));

db/write.c write_f( · line 132

Quoted 10 lines
db/write.c +10 -0
+			return 0;
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+
+	if (corrupt) {
+		struct xfs_buf_ops nowrite_ops;
+

This may not be an explicit failure, but I'd rather not define this on the stack in the local scope of this branch and then implicitly use it outside of that scope. Could we move the definition and possibility the initialization to the top of the function?

Brian

Quoted 43 lines
db/write.c +10 -0
+		/* Temporarily remove write verifier to write bad data */
+		stashed_ops = iocur_top->bp->b_ops;
+		nowrite_ops.verify_read = stashed_ops->verify_read;
+		nowrite_ops.verify_write = xfs_dummy_verify;
+		iocur_top->bp->b_ops = &nowrite_ops;
+		dbprintf(_("Allowing write of corrupted data\n"));
+	}
 
 	(*pf)(DB_WRITE, cur_typ->fields, argc, argv);
 
+	if (stashed_ops)
+		iocur_top->bp->b_ops = stashed_ops;
+
 	return 0;
 }
 
man/man8/xfs_db.8 +7 -1
index 0764832..46a1f39 100644
@@ -728,7 +728,7 @@ and
 bits respectively, and their string equivalent reported
 (but no modifications are made).
 .TP
-.BI "write [" "field value" "] ..."
+.BI "write [\-c] [" "field value" "] ..."
 Write a value to disk.
 Specific fields can be set in structures (struct mode),
 or a block can be set to data values (data mode),
@@ -746,6 +746,12 @@ contents of the block can be shifted or rotated left or right, or filled
 with a sequence, a constant value, or a random value. In this mode
 .B write
 with no arguments gives more information on the allowed commands.
+.RS 1.0i
+.TP 0.4i
+.B \-c
+Skip write verifiers and CRC recalculation; allows invalid data to be written
+to disk.
+.RE
 .SH TYPES
 This section gives the fields in each structure type and their meanings.
 Note that some types of block cover multiple actual structures,
Quoted 4 lines

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

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

#2 lore raw
Quoted 113 lines · Brian Foster

db/crc.c

Quoted 16 lines · Eric Sandeen

Being able to write corrupt data is handy if we wish to test repair against specific types of corruptions.

Add a "-c" option to the write command which allows this.

Note that this also skips CRC updates; it's not currently possible to write invalid data with a valid CRC; CRC recalculation is intertwined with validation.

---

db/crc.c +0 -0
index ad46c3f..861086a 100644

This depends on the previous crc command which isn't merged yet. It might be worth it to include that again here so it hopefully gets picked up.

Quoted 75 lines
db/crc.c +0 -7
@@ -58,13 +58,6 @@ crc_help(void)
 
 }
 
-void
-xfs_dummy_verify(
-	struct xfs_buf *bp)
-{
-	return;
-}
-
 static int
 crc_f(
 	int		argc,
db/crc.h +0 -1
index 80ecec3..9f44615 100644
@@ -20,4 +20,3 @@ struct field;
 
 extern void	crc_init(void);
 extern void	crc_struct(const field_t *fields, int argc, char **argv);
-extern void	xfs_dummy_verify(struct xfs_buf *bp);
db/io.c +7 -0
index eb3daa1..2d18d56 100644
@@ -458,6 +458,13 @@ write_cur_bbs(void)
 }
 
 void
+xfs_dummy_verify(
+	struct xfs_buf *bp)
+{
+	return;
+}
+
+void
 write_cur(void)
 {
 	int skip_crc = (iocur_top->bp->b_ops->verify_write == xfs_dummy_verify);
db/io.h +1 -0
index 71082e6..31d96b4 100644
@@ -63,6 +63,7 @@ extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
 			bbmap_t *bbmap);
 extern void     ring_add(void);
 extern void	set_iocur_type(const struct typ *t);
+extern void	xfs_dummy_verify(struct xfs_buf *bp);
 
 /*
  * returns -1 for unchecked, 0 for bad and 1 for good
db/write.c +2 -1
index a0f14f4..a01c25a 100644
@@ -38,7 +38,7 @@ static int	write_f(int argc, char **argv);
 static void     write_help(void);
 
 static const cmdinfo_t	write_cmd =
-	{ "write", NULL, write_f, 0, -1, 0, N_("[field or value]..."),
+	{ "write", NULL, write_f, 0, -1, 0, N_("[-c] [field or value]..."),
 	  N_("write value to disk"), write_help };
 
 void
@@ -79,6 +79,7 @@ write_help(void)
 "  String mode: 'write \"This_is_a_filename\" - write null terminated string.\n"
 "\n"
 " In data mode type 'write' by itself for a list of specific commands.\n\n"
+" Specifying the -c option will allow writes of invalid (corrupt) data.\n\n"
 ));
 
 }

db/write.c write_f( · line 96

Quoted 6 lines
db/write.c +3 -0
@@ -90,6 +91,9 @@ write_f(
 {
 	pfunc_t	pf;
 	extern char *progname;
+	int c;
+	int corrupt = 0;		/* Allow write of corrupt data; skip verification */
+	const struct xfs_buf_ops *stashed_ops = NULL; 

Trailing space here.

Quoted 7 lines
db/write.c +0 -0
 	if (x.isreadonly & LIBXFS_ISREADONLY) {
 		dbprintf(_("%s started in read only mode, writing disabled\n"),
@@ -109,12 +113,36 @@ write_f(
 		return 0;
 	}
 
Quoted 7 lines

db/write.c write_f( · line 116

Quoted 4 lines
db/write.c +1 -3
-	/* move past the "write" command */
-	argc--;
-	argv++;
+	if (argc) while ((c = getopt(argc, argv, "c")) != EOF) {

Isn't argc always going to be non-zero at this point (e.g., argv[0] == "write")?

Hm, yeah, not sure if that was a thinko or a typo, thanks.

(hm, db's bmap_f does the same thing, I might have just blindly copied that)

Quoted 8 lines

db/write.c write_f( · line 132

Quoted 3 lines
db/write.c +3 -0
+	if (corrupt) {
+		struct xfs_buf_ops nowrite_ops;
+

This may not be an explicit failure, but I'd rather not define this on the stack in the local scope of this branch and then implicitly use it outside of that scope. Could we move the definition and possibility the initialization to the top of the function?

*nod* I'll fix that too.

I'll send the crc patch & this in another 2-patch series (though I am wondering if the crc patch is needed, if we're able to write bad data; it kind of achieves the same goal - thoughts?)

-Eric

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

#3 lore raw
Quoted 135 lines · Eric Sandeen
Quoted 113 lines · Brian Foster

db/crc.c

Quoted 16 lines · Eric Sandeen

Being able to write corrupt data is handy if we wish to test repair against specific types of corruptions.

Add a "-c" option to the write command which allows this.

Note that this also skips CRC updates; it's not currently possible to write invalid data with a valid CRC; CRC recalculation is intertwined with validation.

---

db/crc.c +0 -0
index ad46c3f..861086a 100644

This depends on the previous crc command which isn't merged yet. It might be worth it to include that again here so it hopefully gets picked up.

Quoted 75 lines
db/crc.c +0 -7
@@ -58,13 +58,6 @@ crc_help(void)
 
 }
 
-void
-xfs_dummy_verify(
-	struct xfs_buf *bp)
-{
-	return;
-}
-
 static int
 crc_f(
 	int		argc,
db/crc.h +0 -1
index 80ecec3..9f44615 100644
@@ -20,4 +20,3 @@ struct field;
 
 extern void	crc_init(void);
 extern void	crc_struct(const field_t *fields, int argc, char **argv);
-extern void	xfs_dummy_verify(struct xfs_buf *bp);
db/io.c +7 -0
index eb3daa1..2d18d56 100644
@@ -458,6 +458,13 @@ write_cur_bbs(void)
 }
 
 void
+xfs_dummy_verify(
+	struct xfs_buf *bp)
+{
+	return;
+}
+
+void
 write_cur(void)
 {
 	int skip_crc = (iocur_top->bp->b_ops->verify_write == xfs_dummy_verify);
db/io.h +1 -0
index 71082e6..31d96b4 100644
@@ -63,6 +63,7 @@ extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
 			bbmap_t *bbmap);
 extern void     ring_add(void);
 extern void	set_iocur_type(const struct typ *t);
+extern void	xfs_dummy_verify(struct xfs_buf *bp);
 
 /*
  * returns -1 for unchecked, 0 for bad and 1 for good
db/write.c +2 -1
index a0f14f4..a01c25a 100644
@@ -38,7 +38,7 @@ static int	write_f(int argc, char **argv);
 static void     write_help(void);
 
 static const cmdinfo_t	write_cmd =
-	{ "write", NULL, write_f, 0, -1, 0, N_("[field or value]..."),
+	{ "write", NULL, write_f, 0, -1, 0, N_("[-c] [field or value]..."),
 	  N_("write value to disk"), write_help };
 
 void
@@ -79,6 +79,7 @@ write_help(void)
 "  String mode: 'write \"This_is_a_filename\" - write null terminated string.\n"
 "\n"
 " In data mode type 'write' by itself for a list of specific commands.\n\n"
+" Specifying the -c option will allow writes of invalid (corrupt) data.\n\n"
 ));
 
 }

db/write.c write_f( · line 96

Quoted 6 lines
db/write.c +3 -0
@@ -90,6 +91,9 @@ write_f(
 {
 	pfunc_t	pf;
 	extern char *progname;
+	int c;
+	int corrupt = 0;		/* Allow write of corrupt data; skip verification */
+	const struct xfs_buf_ops *stashed_ops = NULL; 

Trailing space here.

Quoted 7 lines
db/write.c +0 -0
 	if (x.isreadonly & LIBXFS_ISREADONLY) {
 		dbprintf(_("%s started in read only mode, writing disabled\n"),
@@ -109,12 +113,36 @@ write_f(
 		return 0;
 	}
 
Quoted 7 lines

db/write.c write_f( · line 116

Quoted 4 lines
db/write.c +1 -3
-	/* move past the "write" command */
-	argc--;
-	argv++;
+	if (argc) while ((c = getopt(argc, argv, "c")) != EOF) {

Isn't argc always going to be non-zero at this point (e.g., argv[0] == "write")?

Hm, yeah, not sure if that was a thinko or a typo, thanks.

(hm, db's bmap_f does the same thing, I might have just blindly copied that)

Quoted 8 lines

db/write.c write_f( · line 132

Quoted 3 lines
db/write.c +3 -0
+	if (corrupt) {
+		struct xfs_buf_ops nowrite_ops;
+

This may not be an explicit failure, but I'd rather not define this on the stack in the local scope of this branch and then implicitly use it outside of that scope. Could we move the definition and possibility the initialization to the top of the function?

Quoted 6 lines

*nod* I'll fix that too.

I'll send the crc patch & this in another 2-patch series (though I am wondering if the crc patch is needed, if we're able to write bad data; it kind of achieves the same goal - thoughts?)

I suppose that makes sense, but I forget the specifics of the crc command. Did it just munge the crc? Can we accomplish the same thing with this command (e.g., can we write the crc field itself)? If not, I guess it doesn't hurt to have the ability to corrupt a data structure as well the crc itself, where the data structure could be otherwise valid.

Brian

Quoted 6 lines

-Eric

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

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