* [PATCH 1/8] man: list xfs_io lsattr inode flag letters
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
@ 2020-01-24 0:16 ` Darrick J. Wong
2020-01-25 23:16 ` Christoph Hellwig
2020-01-24 0:16 ` [PATCH 2/8] man: document the xfs_db btheight command Darrick J. Wong
` (9 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-24 0:16 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
The section of the xfs_io manpage for the 'chattr' command says to refer
to xfsctl(3) for information on the flags. The inode flag information
was moved to ioctl_xfs_fssetxattr(2) ages ago, and it never actually
mapped the inode flag letters to inode flag bits, so fix the link and
add such a mapping to the xfs_io manpage.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
man/man8/xfs_io.8 | 89 +++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 83 insertions(+), 6 deletions(-)
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index c69b295d..f5431a8c 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -794,18 +794,95 @@ for all directory entries below the currently open file
can be used to restrict the output to directories only).
This is a depth first descent, it does not follow symlinks and
it also does not cross mount points.
+
+The current inode flag letters are documented below.
+Please refer to the
+.BR ioctl_xfs_fsgetxattr "(2)"
+documentation for more details about what they mean.
+
+.PD 0
+.RS
+.TP 0.5i
+.B r
+realtime file (XFS_XFLAG_REALTIME)
+
+.TP
+.B p
+prealloc (XFS_XFLAG_PREALLOC)
+
+.TP
+.B i
+immutable (XFS_XFLAG_IMMUTABLE)
+
+.TP
+.B a
+append only (XFS_XFLAG_APPEND)
+
+.TP
+.B s
+synchronous file writes (XFS_XFLAG_SYNC)
+
+.TP
+.B A
+noatime (XFS_XFLAG_NOATIME)
+
+.TP
+.B d
+nodump (XFS_XFLAG_NODUMP)
+
+.TP
+.B t
+inherit realtime flag (XFS_XFLAG_RTINHERIT)"
+
+.TP
+.B P
+inherit project id (XFS_XFLAG_PROJINHERIT)
+
+.TP
+.B n
+no symlink creation (XFS_XFLAG_NOSYMLINKS)
+
+.TP
+.B e
+extent size hint (XFS_XFLAG_EXTSIZE)
+
+.TP
+.B E
+inherit extent size hint (XFS_XFLAG_EXTSZINHERIT)
+
+.TP
+.B f
+nodefrag (XFS_XFLAG_NODEFRAG)
+
+.TP
+.B S
+filestream allocator (XFS_XFLAG_FILESTREAM)
+
+.TP
+.B x
+direct access persistent memory (XFS_XFLAG_DAX)
+
+.TP
+.B C
+copy on write extent hint (XFS_XFLAG_COWEXTSIZE)
+
+.TP
+.B X
+has extended attributes (XFS_XFLAG_HASATTR)
+.RE
+
.TP
.BR chattr " [ " \-R " | " \-D " ] [ " + / \-riasAdtPneEfSxC " ]"
Change extended inode flags on the currently open file. The
.B \-R
and
.B \-D
-options have the same meaning as above. The mapping between each
-letter and the inode flags (refer to
-.BR xfsctl (3)
-for the full list) is available via the
-.B help
-command.
+options have the same meaning as above.
+
+See the
+.B lsattr
+command above for the list of inode flag letters.
+
.TP
.BI "flink " path
Link the currently open file descriptor into the filesystem namespace.
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 2/8] man: document the xfs_db btheight command
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
2020-01-24 0:16 ` [PATCH 1/8] man: list xfs_io lsattr inode flag letters Darrick J. Wong
@ 2020-01-24 0:16 ` Darrick J. Wong
2020-01-25 23:16 ` Christoph Hellwig
2020-01-24 0:16 ` [PATCH 3/8] man: reformat xfs_quota commands in the manpage for testing Darrick J. Wong
` (8 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-24 0:16 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Document the btheight command in xfs_db.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
man/man8/xfs_db.8 | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
index a1ee3514..53e34983 100644
--- a/man/man8/xfs_db.8
+++ b/man/man8/xfs_db.8
@@ -346,6 +346,45 @@ If the cursor points at an inode, dump the extended attribute block mapping btre
Dump all keys and pointers in intermediate btree nodes, and all records in leaf btree nodes.
.RE
.TP
+.BI "btheight [\-b " blksz "] [\-n " recs "] [\-w " max "|\-w " min "] btree types..."
+For a given number of btree records and a btree type, report the number of
+records and blocks for each level of the btree, and the total number of blocks.
+The btree type must be given after the options.
+
+A raw btree geometry can be provided in the format
+"record_bytes:key_bytes:ptr_bytes:header_type", where header_type is one of
+"short", "long", "shortcrc", or "longcrc".
+
+The supported btree types are:
+.IR bnobt ,
+.IR cntbt ,
+.IR inobt ,
+.IR finobt ,
+.IR bmapbt ,
+.IR refcountbt ,
+and
+.IR rmapbt .
+
+Options are as follows:
+.RS 1.0i
+.TP 0.4i
+.B \-b
+is used to override the btree block size.
+The default is the filesystem block size.
+.TP
+.B \-n
+is used to specify the number of records to store.
+This argument is required.
+.TP
+.B \-w max
+shows only the best case scenario, which is when the btree blocks are
+maximally loaded.
+.TP
+.B \-w min
+shows only the worst case scenario, which is when the btree blocks are
+half full.
+.RE
+.TP
.B check
See the
.B blockget
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 3/8] man: reformat xfs_quota commands in the manpage for testing
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
2020-01-24 0:16 ` [PATCH 1/8] man: list xfs_io lsattr inode flag letters Darrick J. Wong
2020-01-24 0:16 ` [PATCH 2/8] man: document the xfs_db btheight command Darrick J. Wong
@ 2020-01-24 0:16 ` Darrick J. Wong
2020-01-25 23:16 ` Christoph Hellwig
2020-01-24 0:16 ` [PATCH 4/8] man: document some missing xfs_db commands Darrick J. Wong
` (7 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-24 0:16 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Reformat the xfs_quota commands listed in the xfs_quota.8 manpage so
that we can implement a fstest that checks that each command actually
has a section in the manpage.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
man/man8/xfs_quota.8 | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/man/man8/xfs_quota.8 b/man/man8/xfs_quota.8
index d9c5f649..e6fe7cd1 100644
--- a/man/man8/xfs_quota.8
+++ b/man/man8/xfs_quota.8
@@ -228,8 +228,7 @@ option sends the output to
.I file
instead of stdout.
.HP
-.B
-free
+.B free
[
.B \-bir
] [
@@ -398,8 +397,7 @@ option reports information without the header line. The
.B \-t
option performs a terse report.
.HP
-.B
-state
+.B state
[
.B \-gpu
] [
@@ -420,8 +418,7 @@ instead of stdout. The
.B \-a
option reports state on all filesystems and not just the current path.
.HP
-.B
-limit
+.B limit
[
.BR \-g " | " \-p " | " \-u
]
@@ -519,8 +516,7 @@ identified by the current path.
Quota must not be enabled on the filesystem, else this operation will
report an error.
.HP
-.B
-dump
+.B dump
[
.BR \-g " | " \-p " | " \-u
] [
@@ -547,8 +543,7 @@ The file must be in the format produced by the
.B dump
command.
.HP
-.B
-quot
+.B quot
[
.BR \-g " | " \-p " | " \-u
] [
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 4/8] man: document some missing xfs_db commands
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
` (2 preceding siblings ...)
2020-01-24 0:16 ` [PATCH 3/8] man: reformat xfs_quota commands in the manpage for testing Darrick J. Wong
@ 2020-01-24 0:16 ` Darrick J. Wong
2020-01-25 23:16 ` Christoph Hellwig
2020-01-24 0:17 ` [PATCH 5/8] xfs_db: dump per-AG reservations Darrick J. Wong
` (6 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-24 0:16 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
The 'attr_set', 'attr_remove', and 'logformat' commands in xfs_db were
not documented. Add sections about them to the manpage.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
db/attrset.c | 4 ++--
man/man8/xfs_db.8 | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/db/attrset.c b/db/attrset.c
index 56972506..8eecf465 100644
--- a/db/attrset.c
+++ b/db/attrset.c
@@ -23,11 +23,11 @@ static void attrset_help(void);
static const cmdinfo_t attr_set_cmd =
{ "attr_set", "aset", attr_set_f, 1, -1, 0,
- N_("[-r|-s|-p|-u] [-n] [-R|-C] [-v n] name"),
+ N_("[-r|-s|-u] [-n] [-R|-C] [-v n] name"),
N_("set the named attribute on the current inode"), attrset_help };
static const cmdinfo_t attr_remove_cmd =
{ "attr_remove", "aremove", attr_remove_f, 1, -1, 0,
- N_("[-r|-s|-p|-u] [-n] name"),
+ N_("[-r|-s|-u] [-n] name"),
N_("remove the named attribute from the current inode"), attrset_help };
static void
diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
index 53e34983..9f1ff761 100644
--- a/man/man8/xfs_db.8
+++ b/man/man8/xfs_db.8
@@ -179,6 +179,57 @@ Set current address to the AGI block for allocation group
.IR agno .
If no argument is given, use the current allocation group.
.TP
+.BI "attr_remove [\-r|\-u|\-s] [\-n] " name
+Remove the specified extended attribute from the current file.
+.RS 1.0i
+.TP 0.4i
+.B \-r
+Sets the attribute in the root namespace.
+Only one namespace option can be specified.
+.TP
+.B \-u
+Sets the attribute in the user namespace.
+Only one namespace option can be specified.
+.TP
+.B \-s
+Sets the attribute in the secure namespace.
+Only one namespace option can be specified.
+.TP
+.B \-n
+Do not enable 'noattr2' mode on V4 filesystems.
+.RE
+.TP
+.BI "attr_set [\-r|\-u|\-s] [\-n] [\-R|\-C] [\-v " namelen "] " name
+Sets an extended attribute on the current file with the given name.
+.RS 1.0i
+.TP 0.4i
+.B \-r
+Sets the attribute in the root namespace.
+Only one namespace option can be specified.
+.TP
+.B \-u
+Sets the attribute in the user namespace.
+Only one namespace option can be specified.
+.TP
+.B \-s
+Sets the attribute in the secure namespace.
+Only one namespace option can be specified.
+.TP
+.B \-n
+Do not enable 'noattr2' mode on V4 filesystems.
+.TP
+.B \-R
+Replace the attribute.
+The command will fail if the attribute does not already exist.
+.TP
+.B \-C
+Create the attribute.
+The command will fail if the attribute already exists.
+.TP
+.B \-v
+Set the attribute value to a string of this length containing the letter 'v'.
+.RE
+.TP
.B b
See the
.B back
@@ -737,6 +788,13 @@ Start logging output to
.IR filename ,
stop logging, or print the current logging status.
.TP
+.BI "logformat [\-c " cycle "] [\-s " sunit "]"
+Reformats the log to the specified log cycle and log stripe unit.
+This has the effect of clearing the log destructively.
+If the log cycle is not specified, the log is reformatted to the current cycle.
+If the log stripe unit is not specified, the stripe unit from the filesystem
+superblock is used.
+.TP
.B logres
Print transaction reservation size information for each transaction type.
This makes it easier to find discrepancies in the reservation calculations
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 5/8] xfs_db: dump per-AG reservations
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
` (3 preceding siblings ...)
2020-01-24 0:16 ` [PATCH 4/8] man: document some missing xfs_db commands Darrick J. Wong
@ 2020-01-24 0:17 ` Darrick J. Wong
2020-01-25 23:17 ` Christoph Hellwig
2020-01-24 0:17 ` [PATCH 6/8] xfs_io: fix copy_file_range length argument overflow Darrick J. Wong
` (5 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-24 0:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Add a new 'agresv' command to print the size and free blocks count of an
AG along with the size and usage of the per-AG reservation. This
command can be used to aid in diagnosing why a particular filesystem
fails the mount time per-AG space reservation, and to figure out how
much space needs to be freed from a given AG to fix the problem.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
db/info.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++
libxfs/libxfs_api_defs.h | 5 ++
man/man8/xfs_db.8 | 5 ++
3 files changed, 114 insertions(+)
diff --git a/db/info.c b/db/info.c
index e5f1c2dd..fc5ccfe7 100644
--- a/db/info.c
+++ b/db/info.c
@@ -8,6 +8,7 @@
#include "init.h"
#include "output.h"
#include "libfrog/fsgeom.h"
+#include "libfrog/logging.h"
static void
info_help(void)
@@ -45,8 +46,111 @@ static const struct cmdinfo info_cmd = {
.help = info_help,
};
+static void
+agresv_help(void)
+{
+ dbprintf(_(
+"\n"
+" Print the size and per-AG reservation information some allocation groups.\n"
+"\n"
+" Specific allocation group numbers can be provided as command line arguments.\n"
+" If no arguments are provided, all allocation groups are iterated.\n"
+"\n"
+));
+
+}
+
+static void
+print_agresv_info(
+ xfs_agnumber_t agno)
+{
+ struct xfs_buf *bp;
+ struct xfs_agf *agf;
+ xfs_extlen_t ask = 0;
+ xfs_extlen_t used = 0;
+ xfs_extlen_t free = 0;
+ xfs_extlen_t length = 0;
+ int error;
+
+ error = -libxfs_refcountbt_calc_reserves(mp, NULL, agno, &ask, &used);
+ if (error)
+ xfrog_perror(error, "refcountbt");
+ error = -libxfs_finobt_calc_reserves(mp, NULL, agno, &ask, &used);
+ if (error)
+ xfrog_perror(error, "finobt");
+ error = -libxfs_rmapbt_calc_reserves(mp, NULL, agno, &ask, &used);
+ if (error)
+ xfrog_perror(error, "rmapbt");
+
+ error = -libxfs_read_agf(mp, NULL, agno, 0, &bp);
+ if (error)
+ xfrog_perror(error, "AGF");
+ agf = XFS_BUF_TO_AGF(bp);
+ length = be32_to_cpu(agf->agf_length);
+ free = be32_to_cpu(agf->agf_freeblks) +
+ be32_to_cpu(agf->agf_flcount);
+ libxfs_putbuf(bp);
+
+ printf("AG %d: length: %u free: %u reserved: %u used: %u",
+ agno, length, free, ask, used);
+ if (ask - used > free)
+ printf(" <not enough space>");
+ printf("\n");
+}
+
+static int
+agresv_f(
+ int argc,
+ char **argv)
+{
+ xfs_agnumber_t agno;
+ int i;
+
+ if (argc > 1) {
+ for (i = 1; i < argc; i++) {
+ long a;
+ char *p;
+
+ errno = 0;
+ a = strtol(argv[i], &p, 0);
+ if (p == argv[i])
+ errno = ERANGE;
+ if (errno) {
+ perror(argv[i]);
+ continue;
+ }
+
+ if (a < 0 || a >= mp->m_sb.sb_agcount) {
+ fprintf(stderr, "%ld: Not a AG.\n", a);
+ continue;
+ }
+
+ print_agresv_info(a);
+ }
+ return 0;
+ }
+
+ for (agno = 0; agno < mp->m_sb.sb_agcount; agno++)
+ print_agresv_info(agno);
+
+ return 0;
+}
+
+static const struct cmdinfo agresv_cmd = {
+ .name = "agresv",
+ .altname = NULL,
+ .cfunc = agresv_f,
+ .argmin = 0,
+ .argmax = -1,
+ .canpush = 0,
+ .args = NULL,
+ .oneline = N_("print AG reservation stats"),
+ .help = agresv_help,
+};
+
void
info_init(void)
{
add_command(&info_cmd);
+ add_command(&agresv_cmd);
}
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index eed63ace..cc7304ad 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -173,4 +173,9 @@
#define xfs_ag_init_headers libxfs_ag_init_headers
#define xfs_buf_delwri_submit libxfs_buf_delwri_submit
+#define xfs_refcountbt_calc_reserves libxfs_refcountbt_calc_reserves
+#define xfs_finobt_calc_reserves libxfs_finobt_calc_reserves
+#define xfs_rmapbt_calc_reserves libxfs_rmapbt_calc_reserves
+#define xfs_read_agf libxfs_read_agf
+
#endif /* __LIBXFS_API_DEFS_H__ */
diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
index 9f1ff761..7f73d458 100644
--- a/man/man8/xfs_db.8
+++ b/man/man8/xfs_db.8
@@ -179,6 +179,11 @@ Set current address to the AGI block for allocation group
.IR agno .
If no argument is given, use the current allocation group.
.TP
+.BI "agresv [" agno ]
+Displays the length, free block count, per-AG reservation size, and per-AG
+reservation usage for a given AG.
+If no argument is given, display information for all AGs.
+.TP
.BI "attr_remove [\-r|\-u|\-s] [\-n] " name
Remove the specified extended attribute from the current file.
.RS 1.0i
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 6/8] xfs_io: fix copy_file_range length argument overflow
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
` (4 preceding siblings ...)
2020-01-24 0:17 ` [PATCH 5/8] xfs_db: dump per-AG reservations Darrick J. Wong
@ 2020-01-24 0:17 ` Darrick J. Wong
2020-01-25 23:18 ` Christoph Hellwig
2020-01-24 0:17 ` [PATCH 7/8] xfs_io: fix pwrite/pread length truncation on 32-bit systems Darrick J. Wong
` (4 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-24 0:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Don't let the length argument overflow size_t. This is mostly a problem
on 32-bit platforms.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
io/copy_file_range.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/io/copy_file_range.c b/io/copy_file_range.c
index 800b98da..fb5702e1 100644
--- a/io/copy_file_range.c
+++ b/io/copy_file_range.c
@@ -71,6 +71,7 @@ copy_range_f(int argc, char **argv)
{
long long src_off = 0;
long long dst_off = 0;
+ long long llen;
size_t len = 0;
bool len_specified = false;
int opt;
@@ -99,11 +100,21 @@ copy_range_f(int argc, char **argv)
}
break;
case 'l':
- len = cvtnum(fsblocksize, fssectsize, optarg);
- if (len == -1LL) {
+ llen = cvtnum(fsblocksize, fssectsize, optarg);
+ if (llen == -1LL) {
printf(_("invalid length -- %s\n"), optarg);
return 0;
}
+ /*
+ * If size_t can't hold what's in llen, report a
+ * length overflow.
+ */
+ if ((size_t)llen != llen) {
+ errno = EOVERFLOW;
+ perror("copy_range");
+ return 0;
+ }
+ len = llen;
len_specified = true;
break;
case 'f':
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 7/8] xfs_io: fix pwrite/pread length truncation on 32-bit systems
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
` (5 preceding siblings ...)
2020-01-24 0:17 ` [PATCH 6/8] xfs_io: fix copy_file_range length argument overflow Darrick J. Wong
@ 2020-01-24 0:17 ` Darrick J. Wong
2020-01-25 23:18 ` Christoph Hellwig
2020-01-24 0:17 ` [PATCH 8/8] xfs_repair: fix totally broken unit conversion in directory invalidation Darrick J. Wong
` (3 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-24 0:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
The pwrite and pread commands in xfs_io accept an operation length that
can be any quantity that fits in a long long int; and loops to handle
the cases where the operation length is larger than the IO buffer.
Weirdly, the do_ functions contain code to shorten the operation to the
IO buffer size but the @count parameter is size_t, which means that for
a large argument on a 32-bit system, we rip off the upper bits of the
length, turning your 8GB write into a 0 byte write, which does nothing.
This was found by running generic/175 and observing that the 8G test
file it creates has zero length after the operation:
wrote 0/8589934592 bytes at offset 0
0.000000 bytes, 0 ops; 0.0001 sec (0.000000 bytes/sec and 0.0000 ops/sec)
Fix this by pushing long long count all the way through the call stack.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
io/pread.c | 4 ++--
io/pwrite.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/io/pread.c b/io/pread.c
index 1b4352be..d52e21d9 100644
--- a/io/pread.c
+++ b/io/pread.c
@@ -164,7 +164,7 @@ static ssize_t
do_preadv(
int fd,
off64_t offset,
- size_t count)
+ long long count)
{
int vecs = 0;
ssize_t oldlen = 0;
@@ -199,7 +199,7 @@ static ssize_t
do_pread(
int fd,
off64_t offset,
- size_t count,
+ long long count,
size_t buffer_size)
{
if (!vectors)
diff --git a/io/pwrite.c b/io/pwrite.c
index ccf14be9..1c28612f 100644
--- a/io/pwrite.c
+++ b/io/pwrite.c
@@ -54,8 +54,8 @@ static ssize_t
do_pwritev(
int fd,
off64_t offset,
- size_t count,
- int pwritev2_flags)
+ long long count,
+ int pwritev2_flags)
{
int vecs = 0;
ssize_t oldlen = 0;
@@ -97,7 +97,7 @@ static ssize_t
do_pwrite(
int fd,
off64_t offset,
- size_t count,
+ long long count,
size_t buffer_size,
int pwritev2_flags)
{
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 7/8] xfs_io: fix pwrite/pread length truncation on 32-bit systems
2020-01-24 0:17 ` [PATCH 7/8] xfs_io: fix pwrite/pread length truncation on 32-bit systems Darrick J. Wong
@ 2020-01-25 23:18 ` Christoph Hellwig
0 siblings, 0 replies; 33+ messages in thread
From: Christoph Hellwig @ 2020-01-25 23:18 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: sandeen, linux-xfs
On Thu, Jan 23, 2020 at 04:17:17PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> The pwrite and pread commands in xfs_io accept an operation length that
> can be any quantity that fits in a long long int; and loops to handle
> the cases where the operation length is larger than the IO buffer.
>
> Weirdly, the do_ functions contain code to shorten the operation to the
> IO buffer size but the @count parameter is size_t, which means that for
> a large argument on a 32-bit system, we rip off the upper bits of the
> length, turning your 8GB write into a 0 byte write, which does nothing.
>
> This was found by running generic/175 and observing that the 8G test
> file it creates has zero length after the operation:
>
> wrote 0/8589934592 bytes at offset 0
> 0.000000 bytes, 0 ops; 0.0001 sec (0.000000 bytes/sec and 0.0000 ops/sec)
>
> Fix this by pushing long long count all the way through the call stack.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 8/8] xfs_repair: fix totally broken unit conversion in directory invalidation
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
` (6 preceding siblings ...)
2020-01-24 0:17 ` [PATCH 7/8] xfs_io: fix pwrite/pread length truncation on 32-bit systems Darrick J. Wong
@ 2020-01-24 0:17 ` Darrick J. Wong
2020-01-25 23:19 ` Christoph Hellwig
2020-01-28 15:56 ` [PATCH 9/8] xfs_io: fix integer over/underflow handling in timespec_from_string Darrick J. Wong
` (2 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-24 0:17 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Your humble author forgot that xfs_dablk_t has the same units as
xfs_fileoff_t, and totally screwed up the directory buffer invalidation
loop in dir_binval. Not only is there an off-by-one error in the loop
conditional, but the unit conversions are wrong.
Fix all this stupidity by adding a for loop macro to take care of these
details for us so that everyone can iterate all logical directory blocks
(xfs_dir2_db_t) that start within a given bmbt record.
The pre-5.5 xfs_da_get_buf implementation mostly hides the off-by-one
error because dir_binval turns on "don't complain if no mapping" mode,
but on dirblocksize > fsblocksize filesystems the incorrect units can
cause us to miss invalidating some blocks, which can lead to other
buffer cache errors later.
Fixes: f9c559f4e4fb4 ("xfs_repair: invalidate dirty dir buffers when we zap a directory")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
repair/phase6.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/repair/phase6.c b/repair/phase6.c
index 76709f73..0874b649 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -1273,7 +1273,7 @@ dir_binval(
struct xfs_ifork *ifp;
struct xfs_da_geometry *geo;
struct xfs_buf *bp;
- xfs_dablk_t dabno, end_dabno;
+ xfs_dablk_t dabno;
int error = 0;
if (ip->i_d.di_format != XFS_DINODE_FMT_EXTENTS &&
@@ -1283,11 +1283,9 @@ dir_binval(
geo = tp->t_mountp->m_dir_geo;
ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
for_each_xfs_iext(ifp, &icur, &rec) {
- dabno = xfs_dir2_db_to_da(geo, rec.br_startoff +
- geo->fsbcount - 1);
- end_dabno = xfs_dir2_db_to_da(geo, rec.br_startoff +
- rec.br_blockcount);
- for (; dabno <= end_dabno; dabno += geo->fsbcount) {
+ for (dabno = roundup(rec.br_startoff, geo->fsbcount);
+ dabno < rec.br_startoff + rec.br_blockcount;
+ dabno += geo->fsbcount) {
bp = NULL;
error = -libxfs_da_get_buf(tp, ip, dabno, &bp,
whichfork);
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 8/8] xfs_repair: fix totally broken unit conversion in directory invalidation
2020-01-24 0:17 ` [PATCH 8/8] xfs_repair: fix totally broken unit conversion in directory invalidation Darrick J. Wong
@ 2020-01-25 23:19 ` Christoph Hellwig
2020-01-26 22:11 ` Darrick J. Wong
0 siblings, 1 reply; 33+ messages in thread
From: Christoph Hellwig @ 2020-01-25 23:19 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: sandeen, linux-xfs
On Thu, Jan 23, 2020 at 04:17:23PM -0800, Darrick J. Wong wrote:
> Fix all this stupidity by adding a for loop macro to take care of these
> details for us so that everyone can iterate all logical directory blocks
> (xfs_dir2_db_t) that start within a given bmbt record.
No more magic macro in here (thankfully). But the rest looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 8/8] xfs_repair: fix totally broken unit conversion in directory invalidation
2020-01-25 23:19 ` Christoph Hellwig
@ 2020-01-26 22:11 ` Darrick J. Wong
2020-01-30 18:35 ` Eric Sandeen
0 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-26 22:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sandeen, linux-xfs
On Sat, Jan 25, 2020 at 03:19:27PM -0800, Christoph Hellwig wrote:
> On Thu, Jan 23, 2020 at 04:17:23PM -0800, Darrick J. Wong wrote:
> > Fix all this stupidity by adding a for loop macro to take care of these
> > details for us so that everyone can iterate all logical directory blocks
> > (xfs_dir2_db_t) that start within a given bmbt record.
>
> No more magic macro in here (thankfully). But the rest looks good:
Oops, yeah... Eric, if you decide to take this patch, could you please
drop that second paragraph about the macro?
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Thank you for the reviews!
--D
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 8/8] xfs_repair: fix totally broken unit conversion in directory invalidation
2020-01-26 22:11 ` Darrick J. Wong
@ 2020-01-30 18:35 ` Eric Sandeen
0 siblings, 0 replies; 33+ messages in thread
From: Eric Sandeen @ 2020-01-30 18:35 UTC (permalink / raw)
To: Darrick J. Wong, Christoph Hellwig; +Cc: linux-xfs
On 1/26/20 4:11 PM, Darrick J. Wong wrote:
> On Sat, Jan 25, 2020 at 03:19:27PM -0800, Christoph Hellwig wrote:
>> On Thu, Jan 23, 2020 at 04:17:23PM -0800, Darrick J. Wong wrote:
>>> Fix all this stupidity by adding a for loop macro to take care of these
>>> details for us so that everyone can iterate all logical directory blocks
>>> (xfs_dir2_db_t) that start within a given bmbt record.
>>
>> No more magic macro in here (thankfully). But the rest looks good:
>
> Oops, yeah... Eric, if you decide to take this patch, could you please
> drop that second paragraph about the macro?
Done
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Thank you for the reviews!
>
> --D
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 9/8] xfs_io: fix integer over/underflow handling in timespec_from_string
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
` (7 preceding siblings ...)
2020-01-24 0:17 ` [PATCH 8/8] xfs_repair: fix totally broken unit conversion in directory invalidation Darrick J. Wong
@ 2020-01-28 15:56 ` Darrick J. Wong
2020-01-30 18:18 ` Christoph Hellwig
2020-01-30 18:13 ` [PATCH 10/8] libxfs: remove duplicate attr function declarations Darrick J. Wong
2020-01-30 18:15 ` [PATCH 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back Darrick J. Wong
10 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-28 15:56 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
When we're filling out the struct timespec, make sure we detect when the
string value cannot be represented by a (potentially 32-bit) seconds
field in struct timespec.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
libxcmd/input.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/libxcmd/input.c b/libxcmd/input.c
index d232d4f3..137856e3 100644
--- a/libxcmd/input.c
+++ b/libxcmd/input.c
@@ -183,19 +183,30 @@ timestr(
int
timespec_from_string(
- const char * secs,
- const char * nsecs,
- struct timespec * ts)
+ const char *secs,
+ const char *nsecs,
+ struct timespec *ts)
{
- char* p;
+ char *p;
+ unsigned long long int ll;
+
if (!secs || !nsecs || !ts)
return 1;
- ts->tv_sec = strtoull(secs, &p, 0);
+
+ ll = strtoull(secs, &p, 0);
if (*p)
return 1;
- ts->tv_nsec = strtoull(nsecs, &p, 0);
+ ts->tv_sec = ll;
+ if ((unsigned long long int)ts->tv_sec != ll)
+ return 1;
+
+ ll = strtoull(nsecs, &p, 0);
if (*p)
return 1;
+ ts->tv_nsec = ll;
+ if ((unsigned long long int)ts->tv_nsec != ll)
+ return 1;
+
return 0;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 10/8] libxfs: remove duplicate attr function declarations
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
` (8 preceding siblings ...)
2020-01-28 15:56 ` [PATCH 9/8] xfs_io: fix integer over/underflow handling in timespec_from_string Darrick J. Wong
@ 2020-01-30 18:13 ` Darrick J. Wong
2020-01-30 18:17 ` Christoph Hellwig
2020-01-30 18:28 ` Eric Sandeen
2020-01-30 18:15 ` [PATCH 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back Darrick J. Wong
10 siblings, 2 replies; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-30 18:13 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
Remove these function declarations since they're in libxfs/xfs_attr.h
and are therefore redundant.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
libxfs/libxfs_priv.h | 8 --------
1 file changed, 8 deletions(-)
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 03edf0d3..fe08f96b 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -607,14 +607,6 @@ static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
}
/* Keep static checkers quiet about nonstatic functions by exporting */
-int xfs_inode_hasattr(struct xfs_inode *ip);
-int xfs_attr_get_ilocked(struct xfs_inode *ip, struct xfs_da_args *args);
-int xfs_attr_get(struct xfs_inode *ip, const unsigned char *name,
- unsigned char **value, int *valuelenp, int flags);
-int xfs_attr_set(struct xfs_inode *dp, const unsigned char *name,
- unsigned char *value, int valuelen, int flags);
-int xfs_attr_remove(struct xfs_inode *dp, const unsigned char *name, int flags);
-
int xfs_rtbuf_get(struct xfs_mount *mp, struct xfs_trans *tp,
xfs_rtblock_t block, int issum, struct xfs_buf **bpp);
int xfs_rtcheck_range(struct xfs_mount *mp, struct xfs_trans *tp,
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 10/8] libxfs: remove duplicate attr function declarations
2020-01-30 18:13 ` [PATCH 10/8] libxfs: remove duplicate attr function declarations Darrick J. Wong
@ 2020-01-30 18:17 ` Christoph Hellwig
2020-01-30 18:28 ` Eric Sandeen
1 sibling, 0 replies; 33+ messages in thread
From: Christoph Hellwig @ 2020-01-30 18:17 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: sandeen, linux-xfs
On Thu, Jan 30, 2020 at 10:13:30AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Remove these function declarations since they're in libxfs/xfs_attr.h
> and are therefore redundant.
Looks fine - in fact I have these removals in my attr series, just
piecemail..
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 10/8] libxfs: remove duplicate attr function declarations
2020-01-30 18:13 ` [PATCH 10/8] libxfs: remove duplicate attr function declarations Darrick J. Wong
2020-01-30 18:17 ` Christoph Hellwig
@ 2020-01-30 18:28 ` Eric Sandeen
2020-01-30 18:40 ` Darrick J. Wong
1 sibling, 1 reply; 33+ messages in thread
From: Eric Sandeen @ 2020-01-30 18:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 1/30/20 12:13 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Remove these function declarations since they're in libxfs/xfs_attr.h
> and are therefore redundant.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Is it worth keeping this exporting hack around to make static checkers
happy if it's just one more thing to keep up to date in userspace?
-Eric
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 10/8] libxfs: remove duplicate attr function declarations
2020-01-30 18:28 ` Eric Sandeen
@ 2020-01-30 18:40 ` Darrick J. Wong
2020-01-30 18:48 ` Eric Sandeen
0 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-30 18:40 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Thu, Jan 30, 2020 at 12:28:43PM -0600, Eric Sandeen wrote:
> On 1/30/20 12:13 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Remove these function declarations since they're in libxfs/xfs_attr.h
> > and are therefore redundant.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> Is it worth keeping this exporting hack around to make static checkers
> happy if it's just one more thing to keep up to date in userspace?
Probably? It depends on how much you like culling known false positives
when you run smatch/sparse against xfsprogs.
(I for one don't mind not having to remember that stuff...)
--D
> -Eric
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 10/8] libxfs: remove duplicate attr function declarations
2020-01-30 18:40 ` Darrick J. Wong
@ 2020-01-30 18:48 ` Eric Sandeen
0 siblings, 0 replies; 33+ messages in thread
From: Eric Sandeen @ 2020-01-30 18:48 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 1/30/20 12:40 PM, Darrick J. Wong wrote:
> On Thu, Jan 30, 2020 at 12:28:43PM -0600, Eric Sandeen wrote:
>> On 1/30/20 12:13 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> Remove these function declarations since they're in libxfs/xfs_attr.h
>>> and are therefore redundant.
>>>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> Is it worth keeping this exporting hack around to make static checkers
>> happy if it's just one more thing to keep up to date in userspace?
>
> Probably? It depends on how much you like culling known false positives
> when you run smatch/sparse against xfsprogs.
>
> (I for one don't mind not having to remember that stuff...)
Ok. I'm wondering how you guys happened to notice the dups.
Seems like
bool xfs_verify_rtbno(struct xfs_mount *mp, xfs_rtblock_t rtbno);
can be removed as well, FWIW.
-Eric
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back
2020-01-24 0:16 [PATCH 0/8] xfsprogs: random fixes Darrick J. Wong
` (9 preceding siblings ...)
2020-01-30 18:13 ` [PATCH 10/8] libxfs: remove duplicate attr function declarations Darrick J. Wong
@ 2020-01-30 18:15 ` Darrick J. Wong
2020-01-30 18:22 ` Christoph Hellwig
2020-01-30 18:46 ` [PATCH v2 " Darrick J. Wong
10 siblings, 2 replies; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-30 18:15 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
In process_longform_attr, we enforce that the root block of the
attribute index must have both forw or back pointers set to zero.
Unfortunately, the code that nulls out the pointers is not aware that
the root block could be in da3 node format.
This leads to corruption of da3 root node blocks because the functions
that convert attr3 leaf headers to and from the ondisk structures
perform some interpretation of firstused on what they think is an attr1
leaf block.
Found by using xfs/402 to fuzz hdr.info.hdr.forw.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
repair/attr_repair.c | 32 ++++++++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 9a44f610..9d2a40f7 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -952,6 +952,33 @@ _("wrong FS UUID, inode %" PRIu64 " attr block %" PRIu64 "\n"),
return 0;
}
+/*
+ * Zap the forw/back links in an attribute block. Be careful, because the
+ * root block could be an attr leaf block or a da node block.
+ */
+static inline void
+clear_attr_forw_back(
+ struct xfs_buf *bp,
+ struct xfs_attr3_icleaf_hdr *leafhdr)
+{
+ struct xfs_mount *mp = bp->b_mount;
+
+ if (leafhdr->magic == XFS_DA_NODE_MAGIC ||
+ leafhdr->magic == XFS_DA3_NODE_MAGIC) {
+ struct xfs_da3_icnode_hdr da3_hdr;
+
+ xfs_da3_node_hdr_from_disk(mp, &da3_hdr, bp->b_addr);
+ da3_hdr.forw = 0;
+ da3_hdr.back = 0;
+ xfs_da3_node_hdr_to_disk(mp, bp->b_addr, &da3_hdr);
+ return;
+ }
+
+ leafhdr->forw = 0;
+ leafhdr->back = 0;
+ xfs_attr3_leaf_hdr_to_disk(mp->m_attr_geo, bp->b_addr, leafhdr);
+}
+
/*
* Start processing for a leaf or fuller btree.
* A leaf directory is one where the attribute fork is too big for
@@ -1028,10 +1055,7 @@ process_longform_attr(
_("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
ino);
repairlinks = 1;
- leafhdr.forw = 0;
- leafhdr.back = 0;
- xfs_attr3_leaf_hdr_to_disk(mp->m_attr_geo,
- leaf, &leafhdr);
+ clear_attr_forw_back(bp, &leafhdr);
} else {
do_warn(
_("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back
2020-01-30 18:15 ` [PATCH 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back Darrick J. Wong
@ 2020-01-30 18:22 ` Christoph Hellwig
2020-01-30 18:31 ` Darrick J. Wong
2020-01-30 18:46 ` [PATCH v2 " Darrick J. Wong
1 sibling, 1 reply; 33+ messages in thread
From: Christoph Hellwig @ 2020-01-30 18:22 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: sandeen, linux-xfs
While this looks functionally correct, I think the structure in
here is weird. In libxfs we usually check that magic number first
and then branch out into helper that deal with the leaf vs node
format. That is we don't do the xfs_attr3_leaf_hdr_from_disk call
for node format attrs, and also check the forward/backward pointers
based on the actual ichdr. Maybe this code should follow that
structure?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back
2020-01-30 18:22 ` Christoph Hellwig
@ 2020-01-30 18:31 ` Darrick J. Wong
0 siblings, 0 replies; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-30 18:31 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sandeen, linux-xfs
On Thu, Jan 30, 2020 at 10:22:30AM -0800, Christoph Hellwig wrote:
> While this looks functionally correct, I think the structure in
> here is weird. In libxfs we usually check that magic number first
> and then branch out into helper that deal with the leaf vs node
> format. That is we don't do the xfs_attr3_leaf_hdr_from_disk call
> for node format attrs, and also check the forward/backward pointers
> based on the actual ichdr. Maybe this code should follow that
> structure?
Yeah, I'll refactor the case blocks into two functions.
--D
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back
2020-01-30 18:15 ` [PATCH 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back Darrick J. Wong
2020-01-30 18:22 ` Christoph Hellwig
@ 2020-01-30 18:46 ` Darrick J. Wong
2020-01-31 6:03 ` Christoph Hellwig
1 sibling, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-01-30 18:46 UTC (permalink / raw)
To: sandeen; +Cc: linux-xfs, Christoph Hellwig
From: Darrick J. Wong <darrick.wong@oracle.com>
In process_longform_attr, we enforce that the root block of the
attribute index must have both forw or back pointers set to zero.
Unfortunately, the code that nulls out the pointers is not aware that
the root block could be in da3 node format.
This leads to corruption of da3 root node blocks because the functions
that convert attr3 leaf headers to and from the ondisk structures
perform some interpretation of firstused on what they think is an attr1
leaf block.
Found by using xfs/402 to fuzz hdr.info.hdr.forw.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: split the da node code into a separate function since that code path
exits out the bottom of the case statement.
---
repair/attr_repair.c | 98 +++++++++++++++++++++++++++++++++++---------------
1 file changed, 68 insertions(+), 30 deletions(-)
diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 9a44f610..8b66a06d 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -952,6 +952,52 @@ _("wrong FS UUID, inode %" PRIu64 " attr block %" PRIu64 "\n"),
return 0;
}
+static int
+process_longform_da_root(
+ struct xfs_mount *mp,
+ xfs_ino_t ino,
+ struct xfs_dinode *dip,
+ struct blkmap *blkmap,
+ int *repair,
+ struct xfs_buf *bp)
+{
+ struct xfs_da3_icnode_hdr da3_hdr;
+ int repairlinks = 0;
+ int error;
+
+ xfs_da3_node_hdr_from_disk(mp, &da3_hdr, bp->b_addr);
+ /*
+ * check sibling pointers in leaf block or root block 0 before
+ * we have to release the btree block
+ */
+ if (da3_hdr.forw != 0 || da3_hdr.back != 0) {
+ if (!no_modify) {
+ do_warn(
+_("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
+ ino);
+
+ repairlinks = 1;
+ da3_hdr.forw = 0;
+ da3_hdr.back = 0;
+ xfs_da3_node_hdr_to_disk(mp, bp->b_addr, &da3_hdr);
+ } else {
+ do_warn(
+_("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
+ }
+ }
+
+ /* must do this now, to release block 0 before the traversal */
+ if ((*repair || repairlinks) && !no_modify) {
+ *repair = 1;
+ libxfs_writebuf(bp, 0);
+ } else
+ libxfs_putbuf(bp);
+ error = process_node_attr(mp, ino, dip, blkmap); /* + repair */
+ if (error)
+ *repair = 0;
+ return error;
+}
+
/*
* Start processing for a leaf or fuller btree.
* A leaf directory is one where the attribute fork is too big for
@@ -975,7 +1021,6 @@ process_longform_attr(
xfs_dahash_t next_hashval;
int repairlinks = 0;
struct xfs_attr3_icleaf_hdr leafhdr;
- int error;
*repair = 0;
@@ -1019,25 +1064,6 @@ process_longform_attr(
leaf = bp->b_addr;
xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
- /* check sibling pointers in leaf block or root block 0 before
- * we have to release the btree block
- */
- if (leafhdr.forw != 0 || leafhdr.back != 0) {
- if (!no_modify) {
- do_warn(
- _("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
- ino);
- repairlinks = 1;
- leafhdr.forw = 0;
- leafhdr.back = 0;
- xfs_attr3_leaf_hdr_to_disk(mp->m_attr_geo,
- leaf, &leafhdr);
- } else {
- do_warn(
- _("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
- }
- }
-
/*
* use magic number to tell us what type of attribute this is.
* it's possible to have a node or leaf attribute in either an
@@ -1046,6 +1072,26 @@ process_longform_attr(
switch (leafhdr.magic) {
case XFS_ATTR_LEAF_MAGIC: /* leaf-form attribute */
case XFS_ATTR3_LEAF_MAGIC:
+ /*
+ * check sibling pointers in leaf block or root block 0 before
+ * we have to release the btree block
+ */
+ if (leafhdr.forw != 0 || leafhdr.back != 0) {
+ if (!no_modify) {
+ do_warn(
+ _("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
+ ino);
+ repairlinks = 1;
+ leafhdr.forw = 0;
+ leafhdr.back = 0;
+ xfs_attr3_leaf_hdr_to_disk(mp->m_attr_geo,
+ leaf, &leafhdr);
+ } else {
+ do_warn(
+ _("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
+ }
+ }
+
if (process_leaf_attr_block(mp, leaf, 0, ino, blkmap,
0, &next_hashval, repair)) {
*repair = 0;
@@ -1058,16 +1104,8 @@ process_longform_attr(
case XFS_DA_NODE_MAGIC: /* btree-form attribute */
case XFS_DA3_NODE_MAGIC:
- /* must do this now, to release block 0 before the traversal */
- if ((*repair || repairlinks) && !no_modify) {
- *repair = 1;
- libxfs_writebuf(bp, 0);
- } else
- libxfs_putbuf(bp);
- error = process_node_attr(mp, ino, dip, blkmap); /* + repair */
- if (error)
- *repair = 0;
- return error;
+ return process_longform_da_root(mp, ino, dip, blkmap, repair,
+ bp);
default:
do_warn(
_("bad attribute leaf magic # %#x for dir ino %" PRIu64 "\n"),
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v2 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back
2020-01-30 18:46 ` [PATCH v2 " Darrick J. Wong
@ 2020-01-31 6:03 ` Christoph Hellwig
2020-02-04 23:14 ` Darrick J. Wong
0 siblings, 1 reply; 33+ messages in thread
From: Christoph Hellwig @ 2020-01-31 6:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: sandeen, linux-xfs, Christoph Hellwig
Looks sensible, but I think we want the helpers for both the node and
leaf case, something like this untested patch:
diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 9a44f610..0c26f0e6 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -952,6 +952,98 @@ _("wrong FS UUID, inode %" PRIu64 " attr block %" PRIu64 "\n"),
return 0;
}
+static int
+process_leaf_da_root(
+ struct xfs_mount *mp,
+ xfs_ino_t ino,
+ struct xfs_dinode *dip,
+ struct blkmap *blkmap,
+ int *repair,
+ struct xfs_buf *bp)
+{
+ struct xfs_attr3_icleaf_hdr leafhdr;
+ xfs_dahash_t next_hashval;
+ int repairlinks = 0;
+
+ /*
+ * Check sibling pointers in block 0 before we have to release the btree
+ * block.
+ */
+ xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, bp->b_addr);
+ if (leafhdr.forw != 0 || leafhdr.back != 0) {
+ if (!no_modify) {
+ do_warn(
+ _("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
+ ino);
+ repairlinks = 1;
+ leafhdr.forw = 0;
+ leafhdr.back = 0;
+ xfs_attr3_leaf_hdr_to_disk(mp->m_attr_geo, bp->b_addr,
+ &leafhdr);
+ } else {
+ do_warn(
+ _("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
+ }
+ }
+
+ if (process_leaf_attr_block(mp, bp->b_addr, 0, ino, blkmap, 0,
+ &next_hashval, repair)) {
+ *repair = 0;
+ /* the block is bad. lose the attribute fork. */
+ libxfs_putbuf(bp);
+ return 1;
+ }
+
+ *repair = *repair || repairlinks;
+ return 0;
+}
+
+static int
+process_node_da_root(
+ struct xfs_mount *mp,
+ xfs_ino_t ino,
+ struct xfs_dinode *dip,
+ struct blkmap *blkmap,
+ int *repair,
+ struct xfs_buf *bp)
+{
+ struct xfs_da3_icnode_hdr da3_hdr;
+ int repairlinks = 0;
+ int error;
+
+ /*
+ * Check sibling pointers in block 0 before we have to release the btree
+ * block.
+ */
+ xfs_da3_node_hdr_from_disk(mp, &da3_hdr, bp->b_addr);
+ if (da3_hdr.forw != 0 || da3_hdr.back != 0) {
+ if (!no_modify) {
+ do_warn(
+_("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
+ ino);
+
+ repairlinks = 1;
+ da3_hdr.forw = 0;
+ da3_hdr.back = 0;
+ xfs_da3_node_hdr_to_disk(mp, bp->b_addr, &da3_hdr);
+ } else {
+ do_warn(
+_("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
+ }
+ }
+
+ /* must do this now, to release block 0 before the traversal */
+ if ((*repair || repairlinks) && !no_modify) {
+ *repair = 1;
+ libxfs_writebuf(bp, 0);
+ } else
+ libxfs_putbuf(bp);
+ error = process_node_attr(mp, ino, dip, blkmap); /* + repair */
+ if (error)
+ *repair = 0;
+ return error;
+}
+
/*
* Start processing for a leaf or fuller btree.
* A leaf directory is one where the attribute fork is too big for
@@ -963,19 +1055,15 @@ _("wrong FS UUID, inode %" PRIu64 " attr block %" PRIu64 "\n"),
*/
static int
process_longform_attr(
- xfs_mount_t *mp,
- xfs_ino_t ino,
- xfs_dinode_t *dip,
- blkmap_t *blkmap,
- int *repair) /* out - 1 if something was fixed */
+ struct xfs_mount *mp,
+ xfs_ino_t ino,
+ struct xfs_dinode *dip,
+ blkmap_t *blkmap,
+ int *repair) /* out - 1 if something was fixed */
{
- xfs_attr_leafblock_t *leaf;
- xfs_fsblock_t bno;
- xfs_buf_t *bp;
- xfs_dahash_t next_hashval;
- int repairlinks = 0;
- struct xfs_attr3_icleaf_hdr leafhdr;
- int error;
+ xfs_fsblock_t bno;
+ struct xfs_buf *bp;
+ struct xfs_da_blkinfo *info;
*repair = 0;
@@ -1015,77 +1103,35 @@ process_longform_attr(
return 1;
}
- /* verify leaf block */
- leaf = bp->b_addr;
- xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
-
- /* check sibling pointers in leaf block or root block 0 before
- * we have to release the btree block
- */
- if (leafhdr.forw != 0 || leafhdr.back != 0) {
- if (!no_modify) {
- do_warn(
- _("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
- ino);
- repairlinks = 1;
- leafhdr.forw = 0;
- leafhdr.back = 0;
- xfs_attr3_leaf_hdr_to_disk(mp->m_attr_geo,
- leaf, &leafhdr);
- } else {
- do_warn(
- _("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
- }
- }
-
/*
* use magic number to tell us what type of attribute this is.
* it's possible to have a node or leaf attribute in either an
* extent format or btree format attribute fork.
*/
- switch (leafhdr.magic) {
+ info = bp->b_addr;
+ switch (be16_to_cpu(info->magic)) {
case XFS_ATTR_LEAF_MAGIC: /* leaf-form attribute */
case XFS_ATTR3_LEAF_MAGIC:
- if (process_leaf_attr_block(mp, leaf, 0, ino, blkmap,
- 0, &next_hashval, repair)) {
- *repair = 0;
- /* the block is bad. lose the attribute fork. */
- libxfs_putbuf(bp);
- return(1);
- }
- *repair = *repair || repairlinks;
- break;
-
+ return process_leaf_da_root(mp, ino, dip, blkmap, repair, bp);
case XFS_DA_NODE_MAGIC: /* btree-form attribute */
case XFS_DA3_NODE_MAGIC:
- /* must do this now, to release block 0 before the traversal */
- if ((*repair || repairlinks) && !no_modify) {
- *repair = 1;
- libxfs_writebuf(bp, 0);
- } else
- libxfs_putbuf(bp);
- error = process_node_attr(mp, ino, dip, blkmap); /* + repair */
- if (error)
- *repair = 0;
- return error;
+ return process_node_da_root(mp, ino, dip, blkmap, repair, bp);
default:
do_warn(
_("bad attribute leaf magic # %#x for dir ino %" PRIu64 "\n"),
- be16_to_cpu(leaf->hdr.info.magic), ino);
+ be16_to_cpu(info->magic), ino);
libxfs_putbuf(bp);
*repair = 0;
- return(1);
+ return 1;
}
if (*repair && !no_modify)
libxfs_writebuf(bp, 0);
else
libxfs_putbuf(bp);
-
- return(0); /* repair may be set */
+ return 0; /* repair may be set */
}
-
static int
xfs_acl_from_disk(
struct xfs_mount *mp,
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v2 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back
2020-01-31 6:03 ` Christoph Hellwig
@ 2020-02-04 23:14 ` Darrick J. Wong
2020-02-05 6:07 ` Christoph Hellwig
0 siblings, 1 reply; 33+ messages in thread
From: Darrick J. Wong @ 2020-02-04 23:14 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sandeen, linux-xfs
On Thu, Jan 30, 2020 at 10:03:15PM -0800, Christoph Hellwig wrote:
> Looks sensible, but I think we want the helpers for both the node and
> leaf case, something like this untested patch:
Ok, I was about to repost with more or less the same code.
--D
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index 9a44f610..0c26f0e6 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -952,6 +952,98 @@ _("wrong FS UUID, inode %" PRIu64 " attr block %" PRIu64 "\n"),
> return 0;
> }
>
> +static int
> +process_leaf_da_root(
> + struct xfs_mount *mp,
> + xfs_ino_t ino,
> + struct xfs_dinode *dip,
> + struct blkmap *blkmap,
> + int *repair,
> + struct xfs_buf *bp)
> +{
> + struct xfs_attr3_icleaf_hdr leafhdr;
> + xfs_dahash_t next_hashval;
> + int repairlinks = 0;
> +
> + /*
> + * Check sibling pointers in block 0 before we have to release the btree
> + * block.
> + */
> + xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, bp->b_addr);
> + if (leafhdr.forw != 0 || leafhdr.back != 0) {
> + if (!no_modify) {
> + do_warn(
> + _("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
> + ino);
> + repairlinks = 1;
> + leafhdr.forw = 0;
> + leafhdr.back = 0;
> + xfs_attr3_leaf_hdr_to_disk(mp->m_attr_geo, bp->b_addr,
> + &leafhdr);
> + } else {
> + do_warn(
> + _("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
> + }
> + }
> +
> + if (process_leaf_attr_block(mp, bp->b_addr, 0, ino, blkmap, 0,
> + &next_hashval, repair)) {
> + *repair = 0;
> + /* the block is bad. lose the attribute fork. */
> + libxfs_putbuf(bp);
> + return 1;
> + }
> +
> + *repair = *repair || repairlinks;
> + return 0;
> +}
> +
> +static int
> +process_node_da_root(
> + struct xfs_mount *mp,
> + xfs_ino_t ino,
> + struct xfs_dinode *dip,
> + struct blkmap *blkmap,
> + int *repair,
> + struct xfs_buf *bp)
> +{
> + struct xfs_da3_icnode_hdr da3_hdr;
> + int repairlinks = 0;
> + int error;
> +
> + /*
> + * Check sibling pointers in block 0 before we have to release the btree
> + * block.
> + */
> + xfs_da3_node_hdr_from_disk(mp, &da3_hdr, bp->b_addr);
> + if (da3_hdr.forw != 0 || da3_hdr.back != 0) {
> + if (!no_modify) {
> + do_warn(
> +_("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
> + ino);
> +
> + repairlinks = 1;
> + da3_hdr.forw = 0;
> + da3_hdr.back = 0;
> + xfs_da3_node_hdr_to_disk(mp, bp->b_addr, &da3_hdr);
> + } else {
> + do_warn(
> +_("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
> + }
> + }
> +
> + /* must do this now, to release block 0 before the traversal */
> + if ((*repair || repairlinks) && !no_modify) {
> + *repair = 1;
> + libxfs_writebuf(bp, 0);
> + } else
> + libxfs_putbuf(bp);
> + error = process_node_attr(mp, ino, dip, blkmap); /* + repair */
> + if (error)
> + *repair = 0;
> + return error;
> +}
> +
> /*
> * Start processing for a leaf or fuller btree.
> * A leaf directory is one where the attribute fork is too big for
> @@ -963,19 +1055,15 @@ _("wrong FS UUID, inode %" PRIu64 " attr block %" PRIu64 "\n"),
> */
> static int
> process_longform_attr(
> - xfs_mount_t *mp,
> - xfs_ino_t ino,
> - xfs_dinode_t *dip,
> - blkmap_t *blkmap,
> - int *repair) /* out - 1 if something was fixed */
> + struct xfs_mount *mp,
> + xfs_ino_t ino,
> + struct xfs_dinode *dip,
> + blkmap_t *blkmap,
> + int *repair) /* out - 1 if something was fixed */
> {
> - xfs_attr_leafblock_t *leaf;
> - xfs_fsblock_t bno;
> - xfs_buf_t *bp;
> - xfs_dahash_t next_hashval;
> - int repairlinks = 0;
> - struct xfs_attr3_icleaf_hdr leafhdr;
> - int error;
> + xfs_fsblock_t bno;
> + struct xfs_buf *bp;
> + struct xfs_da_blkinfo *info;
>
> *repair = 0;
>
> @@ -1015,77 +1103,35 @@ process_longform_attr(
> return 1;
> }
>
> - /* verify leaf block */
> - leaf = bp->b_addr;
> - xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
> -
> - /* check sibling pointers in leaf block or root block 0 before
> - * we have to release the btree block
> - */
> - if (leafhdr.forw != 0 || leafhdr.back != 0) {
> - if (!no_modify) {
> - do_warn(
> - _("clearing forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"),
> - ino);
> - repairlinks = 1;
> - leafhdr.forw = 0;
> - leafhdr.back = 0;
> - xfs_attr3_leaf_hdr_to_disk(mp->m_attr_geo,
> - leaf, &leafhdr);
> - } else {
> - do_warn(
> - _("would clear forw/back pointers in block 0 for attributes in inode %" PRIu64 "\n"), ino);
> - }
> - }
> -
> /*
> * use magic number to tell us what type of attribute this is.
> * it's possible to have a node or leaf attribute in either an
> * extent format or btree format attribute fork.
> */
> - switch (leafhdr.magic) {
> + info = bp->b_addr;
> + switch (be16_to_cpu(info->magic)) {
> case XFS_ATTR_LEAF_MAGIC: /* leaf-form attribute */
> case XFS_ATTR3_LEAF_MAGIC:
> - if (process_leaf_attr_block(mp, leaf, 0, ino, blkmap,
> - 0, &next_hashval, repair)) {
> - *repair = 0;
> - /* the block is bad. lose the attribute fork. */
> - libxfs_putbuf(bp);
> - return(1);
> - }
> - *repair = *repair || repairlinks;
> - break;
> -
> + return process_leaf_da_root(mp, ino, dip, blkmap, repair, bp);
> case XFS_DA_NODE_MAGIC: /* btree-form attribute */
> case XFS_DA3_NODE_MAGIC:
> - /* must do this now, to release block 0 before the traversal */
> - if ((*repair || repairlinks) && !no_modify) {
> - *repair = 1;
> - libxfs_writebuf(bp, 0);
> - } else
> - libxfs_putbuf(bp);
> - error = process_node_attr(mp, ino, dip, blkmap); /* + repair */
> - if (error)
> - *repair = 0;
> - return error;
> + return process_node_da_root(mp, ino, dip, blkmap, repair, bp);
> default:
> do_warn(
> _("bad attribute leaf magic # %#x for dir ino %" PRIu64 "\n"),
> - be16_to_cpu(leaf->hdr.info.magic), ino);
> + be16_to_cpu(info->magic), ino);
> libxfs_putbuf(bp);
> *repair = 0;
> - return(1);
> + return 1;
> }
>
> if (*repair && !no_modify)
> libxfs_writebuf(bp, 0);
> else
> libxfs_putbuf(bp);
> -
> - return(0); /* repair may be set */
> + return 0; /* repair may be set */
> }
>
> -
> static int
> xfs_acl_from_disk(
> struct xfs_mount *mp,
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 11/8] xfs_repair: don't corrupt a attr fork da3 node when clearing forw/back
2020-02-04 23:14 ` Darrick J. Wong
@ 2020-02-05 6:07 ` Christoph Hellwig
0 siblings, 0 replies; 33+ messages in thread
From: Christoph Hellwig @ 2020-02-05 6:07 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, sandeen, linux-xfs
On Tue, Feb 04, 2020 at 03:14:42PM -0800, Darrick J. Wong wrote:
> On Thu, Jan 30, 2020 at 10:03:15PM -0800, Christoph Hellwig wrote:
> > Looks sensible, but I think we want the helpers for both the node and
> > leaf case, something like this untested patch:
>
> Ok, I was about to repost with more or less the same code.
Just send your version as the official patch, especially if you actually
tested it..
^ permalink raw reply [flat|nested] 33+ messages in thread