[PATCH 5.15 CANDIDATE 0/9] xfs stable candidate patches for 5.15.y (part 3)

[PATCH 5.15 CANDIDATE 0/9] xfs stable candidate patches for 5.15.y (part 3)

[Leah Rumancik]

Hi again,

This set contains fixes from 5.16 to 5.17. The normal testing was run
for this set with no regressions found.

I included some fixes for online scrub. I am not sure if this
is in use for 5.15 though so please let me know if these should be
dropped.

Some refactoring patches were included in this set as dependencies:

bf2307b19513 xfs: fold perag loop iteration logic into helper function
    dependency for f1788b5e5ee25bedf00bb4d25f82b93820d61189
f1788b5e5ee2 xfs: rename the next_agno perag iteration variable
    dependency for 8ed004eb9d07a5d6114db3e97a166707c186262d

Thanks,
Leah


Brian Foster (4):
  xfs: fold perag loop iteration logic into helper function
  xfs: rename the next_agno perag iteration variable
  xfs: terminate perag iteration reliably on agcount
  xfs: fix perag reference leak on iteration race with growfs

Dan Carpenter (1):
  xfs: prevent a WARN_ONCE() in xfs_ioc_attr_list()

Darrick J. Wong (4):
  xfs: fix maxlevels comparisons in the btree staging code
  xfs: fix incorrect decoding in xchk_btree_cur_fsbno
  xfs: fix quotaoff mutex usage now that we don't support disabling it
  xfs: fix a bug in the online fsck directory leaf1 bestcount check

 fs/xfs/libxfs/xfs_ag.h            | 36 ++++++++++++++++++-------------
 fs/xfs/libxfs/xfs_btree_staging.c |  4 ++--
 fs/xfs/scrub/dir.c                | 15 +++++++++----
 fs/xfs/scrub/quota.c              |  4 ++--
 fs/xfs/scrub/repair.c             |  3 +++
 fs/xfs/scrub/scrub.c              |  4 ----
 fs/xfs/scrub/scrub.h              |  1 -
 fs/xfs/scrub/trace.c              |  7 +++---
 fs/xfs/xfs_ioctl.c                |  2 +-
 fs/xfs/xfs_ioctl.h                |  5 +++--
 fs/xfs/xfs_qm_syscalls.c          | 11 +---------
 11 files changed, 48 insertions(+), 44 deletions(-)

-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 1/9] xfs: fix maxlevels comparisons in the btree staging code

[Leah Rumancik]

From: "Darrick J. Wong" <djwong@kernel.org>

[ Upstream commit 78e8ec83a404d63dcc86b251f42e4ee8aff27465 ]

The btree geometry computation function has an off-by-one error in that
it does not allow maximally tall btrees (nlevels == XFS_BTREE_MAXLEVELS).
This can result in repairs failing unnecessarily on very fragmented
filesystems.  Subsequent patches to remove MAXLEVELS usage in favor of
the per-btree type computations will make this a much more likely
occurrence.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandan.babu@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/libxfs/xfs_btree_staging.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_btree_staging.c b/fs/xfs/libxfs/xfs_btree_staging.c
index ac9e80152b5c..89c8a1498df1 100644
--- a/fs/xfs/libxfs/xfs_btree_staging.c
+++ b/fs/xfs/libxfs/xfs_btree_staging.c
@@ -662,7 +662,7 @@ xfs_btree_bload_compute_geometry(
 	xfs_btree_bload_ensure_slack(cur, &bbl->node_slack, 1);
 
 	bbl->nr_records = nr_this_level = nr_records;
-	for (cur->bc_nlevels = 1; cur->bc_nlevels < XFS_BTREE_MAXLEVELS;) {
+	for (cur->bc_nlevels = 1; cur->bc_nlevels <= XFS_BTREE_MAXLEVELS;) {
 		uint64_t	level_blocks;
 		uint64_t	dontcare64;
 		unsigned int	level = cur->bc_nlevels - 1;
@@ -724,7 +724,7 @@ xfs_btree_bload_compute_geometry(
 		nr_this_level = level_blocks;
 	}
 
-	if (cur->bc_nlevels == XFS_BTREE_MAXLEVELS)
+	if (cur->bc_nlevels > XFS_BTREE_MAXLEVELS)
 		return -EOVERFLOW;
 
 	bbl->btree_height = cur->bc_nlevels;
-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 2/9] xfs: fold perag loop iteration logic into helper function

[Leah Rumancik]

From: Brian Foster <bfoster@redhat.com>

[ Upstream commit bf2307b195135ed9c95eebb38920d8bd41843092 ]

Fold the loop iteration logic into a helper in preparation for
further fixups. No functional change in this patch.

[backport: dependency for f1788b5e5ee25bedf00bb4d25f82b93820d61189]

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/libxfs/xfs_ag.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 4c6f9045baca..ddb89e10b6ea 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -124,12 +124,22 @@ void xfs_perag_put(struct xfs_perag *pag);
  * for_each_perag_from() because they terminate at sb_agcount where there are
  * no perag structures in tree beyond end_agno.
  */
+static inline struct xfs_perag *
+xfs_perag_next(
+	struct xfs_perag	*pag,
+	xfs_agnumber_t		*next_agno)
+{
+	struct xfs_mount	*mp = pag->pag_mount;
+
+	*next_agno = pag->pag_agno + 1;
+	xfs_perag_put(pag);
+	return xfs_perag_get(mp, *next_agno);
+}
+
 #define for_each_perag_range(mp, next_agno, end_agno, pag) \
 	for ((pag) = xfs_perag_get((mp), (next_agno)); \
 		(pag) != NULL && (next_agno) <= (end_agno); \
-		(next_agno) = (pag)->pag_agno + 1, \
-		xfs_perag_put(pag), \
-		(pag) = xfs_perag_get((mp), (next_agno)))
+		(pag) = xfs_perag_next((pag), &(next_agno)))
 
 #define for_each_perag_from(mp, next_agno, pag) \
 	for_each_perag_range((mp), (next_agno), (mp)->m_sb.sb_agcount, (pag))
-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 3/9] xfs: rename the next_agno perag iteration variable

[Leah Rumancik]

From: Brian Foster <bfoster@redhat.com>

[ Upstream commit f1788b5e5ee25bedf00bb4d25f82b93820d61189 ]

Rename the next_agno variable to be consistent across the several
iteration macros and shorten line length.

[backport: dependency for 8ed004eb9d07a5d6114db3e97a166707c186262d]

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/libxfs/xfs_ag.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index ddb89e10b6ea..134e8635dee1 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -127,22 +127,22 @@ void xfs_perag_put(struct xfs_perag *pag);
 static inline struct xfs_perag *
 xfs_perag_next(
 	struct xfs_perag	*pag,
-	xfs_agnumber_t		*next_agno)
+	xfs_agnumber_t		*agno)
 {
 	struct xfs_mount	*mp = pag->pag_mount;
 
-	*next_agno = pag->pag_agno + 1;
+	*agno = pag->pag_agno + 1;
 	xfs_perag_put(pag);
-	return xfs_perag_get(mp, *next_agno);
+	return xfs_perag_get(mp, *agno);
 }
 
-#define for_each_perag_range(mp, next_agno, end_agno, pag) \
-	for ((pag) = xfs_perag_get((mp), (next_agno)); \
-		(pag) != NULL && (next_agno) <= (end_agno); \
-		(pag) = xfs_perag_next((pag), &(next_agno)))
+#define for_each_perag_range(mp, agno, end_agno, pag) \
+	for ((pag) = xfs_perag_get((mp), (agno)); \
+		(pag) != NULL && (agno) <= (end_agno); \
+		(pag) = xfs_perag_next((pag), &(agno)))
 
-#define for_each_perag_from(mp, next_agno, pag) \
-	for_each_perag_range((mp), (next_agno), (mp)->m_sb.sb_agcount, (pag))
+#define for_each_perag_from(mp, agno, pag) \
+	for_each_perag_range((mp), (agno), (mp)->m_sb.sb_agcount, (pag))
 
 
 #define for_each_perag(mp, agno, pag) \
-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 4/9] xfs: terminate perag iteration reliably on agcount

[Leah Rumancik]

From: Brian Foster <bfoster@redhat.com>

[ Upstream commit 8ed004eb9d07a5d6114db3e97a166707c186262d ]

The for_each_perag_from() iteration macro relies on sb_agcount to
process every perag currently within EOFS from a given starting
point. It's perfectly valid to have perag structures beyond
sb_agcount, however, such as if a growfs is in progress. If a perag
loop happens to race with growfs in this manner, it will actually
attempt to process the post-EOFS perag where ->pag_agno ==
sb_agcount. This is reproduced by xfs/104 and manifests as the
following assert failure in superblock write verifier context:

 XFS: Assertion failed: agno < mp->m_sb.sb_agcount, file: fs/xfs/libxfs/xfs_types.c, line: 22

Update the corresponding macro to only process perags that are
within the current sb_agcount.

Fixes: 58d43a7e3263 ("xfs: pass perags around in fsmap data dev functions")
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/libxfs/xfs_ag.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 134e8635dee1..4585ebb3f450 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -142,7 +142,7 @@ xfs_perag_next(
 		(pag) = xfs_perag_next((pag), &(agno)))
 
 #define for_each_perag_from(mp, agno, pag) \
-	for_each_perag_range((mp), (agno), (mp)->m_sb.sb_agcount, (pag))
+	for_each_perag_range((mp), (agno), (mp)->m_sb.sb_agcount - 1, (pag))
 
 
 #define for_each_perag(mp, agno, pag) \
-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 5/9] xfs: fix perag reference leak on iteration race with growfs

[Leah Rumancik]

From: Brian Foster <bfoster@redhat.com>

[ Upstream commit 892a666fafa19ab04b5e948f6c92f98f1dafb489 ]

The for_each_perag*() set of macros are hacky in that some (i.e.
those based on sb_agcount) rely on the assumption that perag
iteration terminates naturally with a NULL perag at the specified
end_agno. Others allow for the final AG to have a valid perag and
require the calling function to clean up any potential leftover
xfs_perag reference on termination of the loop.

Aside from providing a subtly inconsistent interface, the former
variant is racy with growfs because growfs can create discoverable
post-eofs perags before the final superblock update that completes
the grow operation and increases sb_agcount. This leads to the
following assert failure (reproduced by xfs/104) in the perag free
path during unmount:

 XFS: Assertion failed: atomic_read(&pag->pag_ref) == 0, file: fs/xfs/libxfs/xfs_ag.c, line: 195

This occurs because one of the many for_each_perag() loops in the
code that is expected to terminate with a NULL pag (and thus has no
post-loop xfs_perag_put() check) raced with a growfs and found a
non-NULL post-EOFS perag, but terminated naturally based on the
end_agno check without releasing the post-EOFS perag.

Rework the iteration logic to lift the agno check from the main for
loop conditional to the iteration helper function. The for loop now
purely terminates on a NULL pag and xfs_perag_next() avoids taking a
reference to any perag beyond end_agno in the first place.

Fixes: f250eedcf762 ("xfs: make for_each_perag... a first class citizen")
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/libxfs/xfs_ag.h | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 4585ebb3f450..3f597cad2c33 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -116,30 +116,26 @@ void xfs_perag_put(struct xfs_perag *pag);
 
 /*
  * Perag iteration APIs
- *
- * XXX: for_each_perag_range() usage really needs an iterator to clean up when
- * we terminate at end_agno because we may have taken a reference to the perag
- * beyond end_agno. Right now callers have to be careful to catch and clean that
- * up themselves. This is not necessary for the callers of for_each_perag() and
- * for_each_perag_from() because they terminate at sb_agcount where there are
- * no perag structures in tree beyond end_agno.
  */
 static inline struct xfs_perag *
 xfs_perag_next(
 	struct xfs_perag	*pag,
-	xfs_agnumber_t		*agno)
+	xfs_agnumber_t		*agno,
+	xfs_agnumber_t		end_agno)
 {
 	struct xfs_mount	*mp = pag->pag_mount;
 
 	*agno = pag->pag_agno + 1;
 	xfs_perag_put(pag);
+	if (*agno > end_agno)
+		return NULL;
 	return xfs_perag_get(mp, *agno);
 }
 
 #define for_each_perag_range(mp, agno, end_agno, pag) \
 	for ((pag) = xfs_perag_get((mp), (agno)); \
-		(pag) != NULL && (agno) <= (end_agno); \
-		(pag) = xfs_perag_next((pag), &(agno)))
+		(pag) != NULL; \
+		(pag) = xfs_perag_next((pag), &(agno), (end_agno)))
 
 #define for_each_perag_from(mp, agno, pag) \
 	for_each_perag_range((mp), (agno), (mp)->m_sb.sb_agcount - 1, (pag))
-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 6/9] xfs: fix incorrect decoding in xchk_btree_cur_fsbno

[Leah Rumancik]

From: "Darrick J. Wong" <djwong@kernel.org>

[ Upstream commit 94a14cfd3b6e639c3fb90994ea24e8513f1b0cce ]

During review of subsequent patches, Dave and I noticed that this
function doesn't work quite right -- accessing cur->bc_ino depends on
the ROOT_IN_INODE flag, not LONG_PTRS.  Fix that and the parentheses
isssue.  While we're at it, remove the piece that accesses cur->bc_ag,
because block 0 of an AG is never part of a btree.

Note: This changes the btree scrubber tracepoints behavior -- if the
cursor has no buffer for a certain level, it will always report
NULLFSBLOCK.  It is assumed that anyone tracing the online fsck code
will also be tracing xchk_start/xchk_done or otherwise be aware of what
exactly is being scrubbed.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/scrub/trace.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/scrub/trace.c b/fs/xfs/scrub/trace.c
index c0ef53fe6611..93c13763c15e 100644
--- a/fs/xfs/scrub/trace.c
+++ b/fs/xfs/scrub/trace.c
@@ -24,10 +24,11 @@ xchk_btree_cur_fsbno(
 	if (level < cur->bc_nlevels && cur->bc_bufs[level])
 		return XFS_DADDR_TO_FSB(cur->bc_mp,
 				xfs_buf_daddr(cur->bc_bufs[level]));
-	if (level == cur->bc_nlevels - 1 && cur->bc_flags & XFS_BTREE_LONG_PTRS)
+
+	if (level == cur->bc_nlevels - 1 &&
+	    (cur->bc_flags & XFS_BTREE_ROOT_IN_INODE))
 		return XFS_INO_TO_FSB(cur->bc_mp, cur->bc_ino.ip->i_ino);
-	if (!(cur->bc_flags & XFS_BTREE_LONG_PTRS))
-		return XFS_AGB_TO_FSB(cur->bc_mp, cur->bc_ag.pag->pag_agno, 0);
+
 	return NULLFSBLOCK;
 }
 
-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 7/9] xfs: fix quotaoff mutex usage now that we don't support disabling it

[Leah Rumancik]

From: "Darrick J. Wong" <djwong@kernel.org>

[ Upstream commit 59d7fab2dff96ed2ca732168859489d71fabd33b ]

Prior to commit 40b52225e58c ("xfs: remove support for disabling quota
accounting on a mounted file system"), we used the quotaoff mutex to
protect dquot operations against quotaoff trying to pull down dquots as
part of disabling quota.

Now that we only support turning off quota enforcement, the quotaoff
mutex only protects changes in m_qflags/sb_qflags.  We don't need it to
protect dquots, which means we can remove it from setqlimits and the
dquot scrub code.  While we're at it, fix the function that forces
quotacheck, since it should have been taking the quotaoff mutex.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/scrub/quota.c     |  4 ++--
 fs/xfs/scrub/repair.c    |  3 +++
 fs/xfs/scrub/scrub.c     |  4 ----
 fs/xfs/scrub/scrub.h     |  1 -
 fs/xfs/xfs_qm_syscalls.c | 11 +----------
 5 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/fs/xfs/scrub/quota.c b/fs/xfs/scrub/quota.c
index d6c1b00a4fc8..3c7506c7553c 100644
--- a/fs/xfs/scrub/quota.c
+++ b/fs/xfs/scrub/quota.c
@@ -48,10 +48,10 @@ xchk_setup_quota(
 	dqtype = xchk_quota_to_dqtype(sc);
 	if (dqtype == 0)
 		return -EINVAL;
-	sc->flags |= XCHK_HAS_QUOTAOFFLOCK;
-	mutex_lock(&sc->mp->m_quotainfo->qi_quotaofflock);
+
 	if (!xfs_this_quota_on(sc->mp, dqtype))
 		return -ENOENT;
+
 	error = xchk_setup_fs(sc);
 	if (error)
 		return error;
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 8f3cba14ada3..1e7b6b209ee8 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -25,6 +25,7 @@
 #include "xfs_ag.h"
 #include "xfs_ag_resv.h"
 #include "xfs_quota.h"
+#include "xfs_qm.h"
 #include "scrub/scrub.h"
 #include "scrub/common.h"
 #include "scrub/trace.h"
@@ -912,11 +913,13 @@ xrep_force_quotacheck(
 	if (!(flag & sc->mp->m_qflags))
 		return;
 
+	mutex_lock(&sc->mp->m_quotainfo->qi_quotaofflock);
 	sc->mp->m_qflags &= ~flag;
 	spin_lock(&sc->mp->m_sb_lock);
 	sc->mp->m_sb.sb_qflags &= ~flag;
 	spin_unlock(&sc->mp->m_sb_lock);
 	xfs_log_sb(sc->tp);
+	mutex_unlock(&sc->mp->m_quotainfo->qi_quotaofflock);
 }
 
 /*
diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
index 51e4c61916d2..3e4607dfc661 100644
--- a/fs/xfs/scrub/scrub.c
+++ b/fs/xfs/scrub/scrub.c
@@ -173,10 +173,6 @@ xchk_teardown(
 		mnt_drop_write_file(sc->file);
 	if (sc->flags & XCHK_REAPING_DISABLED)
 		xchk_start_reaping(sc);
-	if (sc->flags & XCHK_HAS_QUOTAOFFLOCK) {
-		mutex_unlock(&sc->mp->m_quotainfo->qi_quotaofflock);
-		sc->flags &= ~XCHK_HAS_QUOTAOFFLOCK;
-	}
 	if (sc->buf) {
 		kmem_free(sc->buf);
 		sc->buf = NULL;
diff --git a/fs/xfs/scrub/scrub.h b/fs/xfs/scrub/scrub.h
index 80e5026bba44..3de5287e98d8 100644
--- a/fs/xfs/scrub/scrub.h
+++ b/fs/xfs/scrub/scrub.h
@@ -88,7 +88,6 @@ struct xfs_scrub {
 
 /* XCHK state flags grow up from zero, XREP state flags grown down from 2^31 */
 #define XCHK_TRY_HARDER		(1 << 0)  /* can't get resources, try again */
-#define XCHK_HAS_QUOTAOFFLOCK	(1 << 1)  /* we hold the quotaoff lock */
 #define XCHK_REAPING_DISABLED	(1 << 2)  /* background block reaping paused */
 #define XREP_ALREADY_FIXED	(1 << 31) /* checking our repair work */
 
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 47fe60e1a887..7d5a31827681 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -302,13 +302,6 @@ xfs_qm_scall_setqlim(
 	if ((newlim->d_fieldmask & XFS_QC_MASK) == 0)
 		return 0;
 
-	/*
-	 * We don't want to race with a quotaoff so take the quotaoff lock.
-	 * We don't hold an inode lock, so there's nothing else to stop
-	 * a quotaoff from happening.
-	 */
-	mutex_lock(&q->qi_quotaofflock);
-
 	/*
 	 * Get the dquot (locked) before we start, as we need to do a
 	 * transaction to allocate it if it doesn't exist. Once we have the
@@ -319,7 +312,7 @@ xfs_qm_scall_setqlim(
 	error = xfs_qm_dqget(mp, id, type, true, &dqp);
 	if (error) {
 		ASSERT(error != -ENOENT);
-		goto out_unlock;
+		return error;
 	}
 
 	defq = xfs_get_defquota(q, xfs_dquot_type(dqp));
@@ -415,8 +408,6 @@ xfs_qm_scall_setqlim(
 
 out_rele:
 	xfs_qm_dqrele(dqp);
-out_unlock:
-	mutex_unlock(&q->qi_quotaofflock);
 	return error;
 }
 
-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 8/9] xfs: fix a bug in the online fsck directory leaf1 bestcount check

[Leah Rumancik]

From: "Darrick J. Wong" <djwong@kernel.org>

[ Upstream commit e5d1802c70f50e0660ee7f598dc2c40312c9e0af ]

When xfs_scrub encounters a directory with a leaf1 block, it tries to
validate that the leaf1 block's bestcount (aka the best free count of
each directory data block) is the correct size.  Previously, this author
believed that comparing bestcount to the directory isize (since
directory data blocks are under isize, and leaf/bestfree blocks are
above it) was sufficient.

Unfortunately during testing of online repair, it was discovered that it
is possible to create a directory with a hole between the last directory
block and isize.  The directory code seems to handle this situation just
fine and xfs_repair doesn't complain, which effectively makes this quirk
part of the disk format.

Fix the check to work properly.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/scrub/dir.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 200a63f58fe7..38897adde7b5 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -497,6 +497,7 @@ STATIC int
 xchk_directory_leaf1_bestfree(
 	struct xfs_scrub		*sc,
 	struct xfs_da_args		*args,
+	xfs_dir2_db_t			last_data_db,
 	xfs_dablk_t			lblk)
 {
 	struct xfs_dir3_icleaf_hdr	leafhdr;
@@ -534,10 +535,14 @@ xchk_directory_leaf1_bestfree(
 	}
 
 	/*
-	 * There should be as many bestfree slots as there are dir data
-	 * blocks that can fit under i_size.
+	 * There must be enough bestfree slots to cover all the directory data
+	 * blocks that we scanned.  It is possible for there to be a hole
+	 * between the last data block and i_disk_size.  This seems like an
+	 * oversight to the scrub author, but as we have been writing out
+	 * directories like this (and xfs_repair doesn't mind them) for years,
+	 * that's what we have to check.
 	 */
-	if (bestcount != xfs_dir2_byte_to_db(geo, sc->ip->i_disk_size)) {
+	if (bestcount != last_data_db + 1) {
 		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, lblk);
 		goto out;
 	}
@@ -669,6 +674,7 @@ xchk_directory_blocks(
 	xfs_fileoff_t		lblk;
 	struct xfs_iext_cursor	icur;
 	xfs_dablk_t		dabno;
+	xfs_dir2_db_t		last_data_db = 0;
 	bool			found;
 	int			is_block = 0;
 	int			error;
@@ -712,6 +718,7 @@ xchk_directory_blocks(
 				args.geo->fsbcount);
 		     lblk < got.br_startoff + got.br_blockcount;
 		     lblk += args.geo->fsbcount) {
+			last_data_db = xfs_dir2_da_to_db(args.geo, lblk);
 			error = xchk_directory_data_bestfree(sc, lblk,
 					is_block);
 			if (error)
@@ -734,7 +741,7 @@ xchk_directory_blocks(
 			xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, lblk);
 			goto out;
 		}
-		error = xchk_directory_leaf1_bestfree(sc, &args,
+		error = xchk_directory_leaf1_bestfree(sc, &args, last_data_db,
 				leaf_lblk);
 		if (error)
 			goto out;
-- 
2.37.0.170.g444d1eabd0-goog

[PATCH 5.15 CANDIDATE 9/9] xfs: prevent a WARN_ONCE() in xfs_ioc_attr_list()

[Leah Rumancik]

From: Dan Carpenter <dan.carpenter@oracle.com>

[ Upstream commit 6ed6356b07714e0198be3bc3ecccc8b40a212de4 ]

The "bufsize" comes from the root user.  If "bufsize" is negative then,
because of type promotion, neither of the validation checks at the start
of the function are able to catch it:

	if (bufsize < sizeof(struct xfs_attrlist) ||
	    bufsize > XFS_XATTR_LIST_MAX)
		return -EINVAL;

This means "bufsize" will trigger (WARN_ON_ONCE(size > INT_MAX)) in
kvmalloc_node().  Fix this by changing the type from int to size_t.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
---
 fs/xfs/xfs_ioctl.c | 2 +-
 fs/xfs/xfs_ioctl.h | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 09269f478df9..fba52e75e98b 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -372,7 +372,7 @@ int
 xfs_ioc_attr_list(
 	struct xfs_inode		*dp,
 	void __user			*ubuf,
-	int				bufsize,
+	size_t				bufsize,
 	int				flags,
 	struct xfs_attrlist_cursor __user *ucursor)
 {
diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
index 28453a6d4461..845d3bcab74b 100644
--- a/fs/xfs/xfs_ioctl.h
+++ b/fs/xfs/xfs_ioctl.h
@@ -38,8 +38,9 @@ xfs_readlink_by_handle(
 int xfs_ioc_attrmulti_one(struct file *parfilp, struct inode *inode,
 		uint32_t opcode, void __user *uname, void __user *value,
 		uint32_t *len, uint32_t flags);
-int xfs_ioc_attr_list(struct xfs_inode *dp, void __user *ubuf, int bufsize,
-	int flags, struct xfs_attrlist_cursor __user *ucursor);
+int xfs_ioc_attr_list(struct xfs_inode *dp, void __user *ubuf,
+		      size_t bufsize, int flags,
+		      struct xfs_attrlist_cursor __user *ucursor);
 
 extern struct dentry *
 xfs_handle_to_dentry(
-- 
2.37.0.170.g444d1eabd0-goog

Re: [PATCH 5.15 CANDIDATE 0/9] xfs stable candidate patches for 5.15.y (part 3)

[Darrick J. Wong]

On Mon, Jul 18, 2022 at 01:29:50PM -0700, Leah Rumancik wrote:
> Hi again,
> 
> This set contains fixes from 5.16 to 5.17. The normal testing was run
> for this set with no regressions found.
> 
> I included some fixes for online scrub. I am not sure if this
> is in use for 5.15 though so please let me know if these should be
> dropped.
> 
> Some refactoring patches were included in this set as dependencies:
> 
> bf2307b19513 xfs: fold perag loop iteration logic into helper function
>     dependency for f1788b5e5ee25bedf00bb4d25f82b93820d61189
> f1788b5e5ee2 xfs: rename the next_agno perag iteration variable
>     dependency for 8ed004eb9d07a5d6114db3e97a166707c186262d
> 
> Thanks,
> Leah
> 
> 
> Brian Foster (4):
>   xfs: fold perag loop iteration logic into helper function
>   xfs: rename the next_agno perag iteration variable
>   xfs: terminate perag iteration reliably on agcount
>   xfs: fix perag reference leak on iteration race with growfs
> 
> Dan Carpenter (1):
>   xfs: prevent a WARN_ONCE() in xfs_ioc_attr_list()
> 
> Darrick J. Wong (4):
>   xfs: fix maxlevels comparisons in the btree staging code

Up to this point,
Acked-by: Darrick J. Wong <djwong@kernel.org>

>   xfs: fix incorrect decoding in xchk_btree_cur_fsbno
>   xfs: fix quotaoff mutex usage now that we don't support disabling it
>   xfs: fix a bug in the online fsck directory leaf1 bestcount check

No objections to these last three, since they're legitimate fixes for
bugs in 5.15, but I would advise y'all not to worry too much about fixes
for EXPERIMENTAL features.

--D

> 
>  fs/xfs/libxfs/xfs_ag.h            | 36 ++++++++++++++++++-------------
>  fs/xfs/libxfs/xfs_btree_staging.c |  4 ++--
>  fs/xfs/scrub/dir.c                | 15 +++++++++----
>  fs/xfs/scrub/quota.c              |  4 ++--
>  fs/xfs/scrub/repair.c             |  3 +++
>  fs/xfs/scrub/scrub.c              |  4 ----
>  fs/xfs/scrub/scrub.h              |  1 -
>  fs/xfs/scrub/trace.c              |  7 +++---
>  fs/xfs/xfs_ioctl.c                |  2 +-
>  fs/xfs/xfs_ioctl.h                |  5 +++--
>  fs/xfs/xfs_qm_syscalls.c          | 11 +---------
>  11 files changed, 48 insertions(+), 44 deletions(-)
> 
> -- 
> 2.37.0.170.g444d1eabd0-goog
> 

Re: [PATCH 5.15 CANDIDATE 0/9] xfs stable candidate patches for 5.15.y (part 3)

[Amir Goldstein]

On Tue, Jul 19, 2022 at 12:05 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Mon, Jul 18, 2022 at 01:29:50PM -0700, Leah Rumancik wrote:
> > Hi again,
> >
> > This set contains fixes from 5.16 to 5.17. The normal testing was run
> > for this set with no regressions found.
> >
> > I included some fixes for online scrub. I am not sure if this
> > is in use for 5.15 though so please let me know if these should be
> > dropped.
> >
> > Some refactoring patches were included in this set as dependencies:
> >
> > bf2307b19513 xfs: fold perag loop iteration logic into helper function
> >     dependency for f1788b5e5ee25bedf00bb4d25f82b93820d61189
> > f1788b5e5ee2 xfs: rename the next_agno perag iteration variable
> >     dependency for 8ed004eb9d07a5d6114db3e97a166707c186262d
> >
> > Thanks,
> > Leah
> >
> >
> > Brian Foster (4):
> >   xfs: fold perag loop iteration logic into helper function
> >   xfs: rename the next_agno perag iteration variable
> >   xfs: terminate perag iteration reliably on agcount
> >   xfs: fix perag reference leak on iteration race with growfs
> >
> > Dan Carpenter (1):
> >   xfs: prevent a WARN_ONCE() in xfs_ioc_attr_list()
> >
> > Darrick J. Wong (4):
> >   xfs: fix maxlevels comparisons in the btree staging code
>
> Up to this point,
> Acked-by: Darrick J. Wong <djwong@kernel.org>
>
> >   xfs: fix incorrect decoding in xchk_btree_cur_fsbno
> >   xfs: fix quotaoff mutex usage now that we don't support disabling it
> >   xfs: fix a bug in the online fsck directory leaf1 bestcount check
>
> No objections to these last three, since they're legitimate fixes for
> bugs in 5.15, but I would advise y'all not to worry too much about fixes
> for EXPERIMENTAL features.

FWIW, from the set above, I only picked Dan Carpenter's fix for 5.10.
I'll include it in one of the following updates.

Thanks,
Amir.

Re: [PATCH 5.15 CANDIDATE 0/9] xfs stable candidate patches for 5.15.y (part 3)

[Darrick J. Wong]

On Tue, Jul 19, 2022 at 10:44:29AM +0200, Amir Goldstein wrote:
> On Tue, Jul 19, 2022 at 12:05 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Mon, Jul 18, 2022 at 01:29:50PM -0700, Leah Rumancik wrote:
> > > Hi again,
> > >
> > > This set contains fixes from 5.16 to 5.17. The normal testing was run
> > > for this set with no regressions found.
> > >
> > > I included some fixes for online scrub. I am not sure if this
> > > is in use for 5.15 though so please let me know if these should be
> > > dropped.
> > >
> > > Some refactoring patches were included in this set as dependencies:
> > >
> > > bf2307b19513 xfs: fold perag loop iteration logic into helper function
> > >     dependency for f1788b5e5ee25bedf00bb4d25f82b93820d61189
> > > f1788b5e5ee2 xfs: rename the next_agno perag iteration variable
> > >     dependency for 8ed004eb9d07a5d6114db3e97a166707c186262d
> > >
> > > Thanks,
> > > Leah
> > >
> > >
> > > Brian Foster (4):
> > >   xfs: fold perag loop iteration logic into helper function
> > >   xfs: rename the next_agno perag iteration variable
> > >   xfs: terminate perag iteration reliably on agcount
> > >   xfs: fix perag reference leak on iteration race with growfs
> > >
> > > Dan Carpenter (1):
> > >   xfs: prevent a WARN_ONCE() in xfs_ioc_attr_list()
> > >
> > > Darrick J. Wong (4):
> > >   xfs: fix maxlevels comparisons in the btree staging code
> >
> > Up to this point,
> > Acked-by: Darrick J. Wong <djwong@kernel.org>
> >
> > >   xfs: fix incorrect decoding in xchk_btree_cur_fsbno
> > >   xfs: fix quotaoff mutex usage now that we don't support disabling it
> > >   xfs: fix a bug in the online fsck directory leaf1 bestcount check
> >
> > No objections to these last three, since they're legitimate fixes for
> > bugs in 5.15, but I would advise y'all not to worry too much about fixes
> > for EXPERIMENTAL features.

Also, to clarify -- if you /do/ want to pick up the scrub fixes, then
yes, the Acked-by above does apply to the entire set.  I don't know if
you have people running (experimental) scrub, but I don't know that you
**don't**. :)

> FWIW, from the set above, I only picked Dan Carpenter's fix for 5.10.
> I'll include it in one of the following updates.

<nod>

--D

> Thanks,
> Amir.

Re: [PATCH 5.15 CANDIDATE 0/9] xfs stable candidate patches for 5.15.y (part 3)

[Leah Rumancik]

On Tue, Jul 19, 2022 at 07:44:25AM -0700, Darrick J. Wong wrote:
> On Tue, Jul 19, 2022 at 10:44:29AM +0200, Amir Goldstein wrote:
> > On Tue, Jul 19, 2022 at 12:05 AM Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > On Mon, Jul 18, 2022 at 01:29:50PM -0700, Leah Rumancik wrote:
> > > > Hi again,
> > > >
> > > > This set contains fixes from 5.16 to 5.17. The normal testing was run
> > > > for this set with no regressions found.
> > > >
> > > > I included some fixes for online scrub. I am not sure if this
> > > > is in use for 5.15 though so please let me know if these should be
> > > > dropped.
> > > >
> > > > Some refactoring patches were included in this set as dependencies:
> > > >
> > > > bf2307b19513 xfs: fold perag loop iteration logic into helper function
> > > >     dependency for f1788b5e5ee25bedf00bb4d25f82b93820d61189
> > > > f1788b5e5ee2 xfs: rename the next_agno perag iteration variable
> > > >     dependency for 8ed004eb9d07a5d6114db3e97a166707c186262d
> > > >
> > > > Thanks,
> > > > Leah
> > > >
> > > >
> > > > Brian Foster (4):
> > > >   xfs: fold perag loop iteration logic into helper function
> > > >   xfs: rename the next_agno perag iteration variable
> > > >   xfs: terminate perag iteration reliably on agcount
> > > >   xfs: fix perag reference leak on iteration race with growfs
> > > >
> > > > Dan Carpenter (1):
> > > >   xfs: prevent a WARN_ONCE() in xfs_ioc_attr_list()
> > > >
> > > > Darrick J. Wong (4):
> > > >   xfs: fix maxlevels comparisons in the btree staging code
> > >
> > > Up to this point,
> > > Acked-by: Darrick J. Wong <djwong@kernel.org>
> > >
> > > >   xfs: fix incorrect decoding in xchk_btree_cur_fsbno
> > > >   xfs: fix quotaoff mutex usage now that we don't support disabling it
> > > >   xfs: fix a bug in the online fsck directory leaf1 bestcount check
> > >
> > > No objections to these last three, since they're legitimate fixes for
> > > bugs in 5.15, but I would advise y'all not to worry too much about fixes
> > > for EXPERIMENTAL features.
> 
> Also, to clarify -- if you /do/ want to pick up the scrub fixes, then
> yes, the Acked-by above does apply to the entire set.  I don't know if
> you have people running (experimental) scrub, but I don't know that you
> **don't**. :)

These fixes aren't a priority over here so I'll postpone scrub fixes in
the future since it doesn't seem like people care. For this set, is
there coverage in xfstests for them? If so, I'll go ahead and keep them,
but if not, I'll just drop them.

- Leah

> 
> > FWIW, from the set above, I only picked Dan Carpenter's fix for 5.10.
> > I'll include it in one of the following updates.
> 
> <nod>
> 
> --D
> 
> > Thanks,
> > Amir.

Re: [PATCH 5.15 CANDIDATE 0/9] xfs stable candidate patches for 5.15.y (part 3)

[Darrick J. Wong]

On Wed, Jul 20, 2022 at 11:11:27AM -0700, Leah Rumancik wrote:
> On Tue, Jul 19, 2022 at 07:44:25AM -0700, Darrick J. Wong wrote:
> > On Tue, Jul 19, 2022 at 10:44:29AM +0200, Amir Goldstein wrote:
> > > On Tue, Jul 19, 2022 at 12:05 AM Darrick J. Wong <djwong@kernel.org> wrote:
> > > >
> > > > On Mon, Jul 18, 2022 at 01:29:50PM -0700, Leah Rumancik wrote:
> > > > > Hi again,
> > > > >
> > > > > This set contains fixes from 5.16 to 5.17. The normal testing was run
> > > > > for this set with no regressions found.
> > > > >
> > > > > I included some fixes for online scrub. I am not sure if this
> > > > > is in use for 5.15 though so please let me know if these should be
> > > > > dropped.
> > > > >
> > > > > Some refactoring patches were included in this set as dependencies:
> > > > >
> > > > > bf2307b19513 xfs: fold perag loop iteration logic into helper function
> > > > >     dependency for f1788b5e5ee25bedf00bb4d25f82b93820d61189
> > > > > f1788b5e5ee2 xfs: rename the next_agno perag iteration variable
> > > > >     dependency for 8ed004eb9d07a5d6114db3e97a166707c186262d
> > > > >
> > > > > Thanks,
> > > > > Leah
> > > > >
> > > > >
> > > > > Brian Foster (4):
> > > > >   xfs: fold perag loop iteration logic into helper function
> > > > >   xfs: rename the next_agno perag iteration variable
> > > > >   xfs: terminate perag iteration reliably on agcount
> > > > >   xfs: fix perag reference leak on iteration race with growfs
> > > > >
> > > > > Dan Carpenter (1):
> > > > >   xfs: prevent a WARN_ONCE() in xfs_ioc_attr_list()
> > > > >
> > > > > Darrick J. Wong (4):
> > > > >   xfs: fix maxlevels comparisons in the btree staging code
> > > >
> > > > Up to this point,
> > > > Acked-by: Darrick J. Wong <djwong@kernel.org>
> > > >
> > > > >   xfs: fix incorrect decoding in xchk_btree_cur_fsbno
> > > > >   xfs: fix quotaoff mutex usage now that we don't support disabling it
> > > > >   xfs: fix a bug in the online fsck directory leaf1 bestcount check
> > > >
> > > > No objections to these last three, since they're legitimate fixes for
> > > > bugs in 5.15, but I would advise y'all not to worry too much about fixes
> > > > for EXPERIMENTAL features.
> > 
> > Also, to clarify -- if you /do/ want to pick up the scrub fixes, then
> > yes, the Acked-by above does apply to the entire set.  I don't know if
> > you have people running (experimental) scrub, but I don't know that you
> > **don't**. :)
> 
> These fixes aren't a priority over here so I'll postpone scrub fixes in
> the future since it doesn't seem like people care. For this set, is
> there coverage in xfstests for them? If so, I'll go ahead and keep them,
> but if not, I'll just drop them.

Nothing other than the general fuzzing tests.  You might as well ignore
them.

--D

> - Leah
> 
> > 
> > > FWIW, from the set above, I only picked Dan Carpenter's fix for 5.10.
> > > I'll include it in one of the following updates.
> > 
> > <nod>
> > 
> > --D
> > 
> > > Thanks,
> > > Amir.