* [PATCH] xfs: bound inode fork length against the fork size during log recovery
@ 2026-07-07 13:58 Aldo Ariel Panzardo
2026-07-07 16:26 ` Darrick J. Wong
2026-07-07 19:02 ` [PATCH v2] " Aldo Ariel Panzardo
0 siblings, 2 replies; 3+ messages in thread
From: Aldo Ariel Panzardo @ 2026-07-07 13:58 UTC (permalink / raw)
To: linux-xfs, Carlos Maiolino
Cc: Darrick J . Wong, Chandan Babu R, linux-kernel,
Aldo Ariel Panzardo
xlog_recover_inode_commit_pass2() copies an inode log item's data/attr
fork region into the inode buffer using the on-log region length without
bounding it against the fork capacity, e.g.:
len = item->ri_buf[2].iov_len;
memcpy(XFS_DFORK_DPTR(dip), src, len);
The only guard is an ASSERT, which is a no-op on production kernels
(CONFIG_XFS_DEBUG off), and xfs_dinode_verify() runs only after the copy.
A crafted image with a dirty log can therefore drive a heap out-of-bounds
write at mount time. The XFS_ILOG_DBROOT sibling already passes
XFS_DFORK_DSIZE as a bound; the DDATA/DEXT and ADATA/AEXT memcpy paths
did not.
Reject the log item with -EFSCORRUPTED when the region length exceeds the
destination fork size.
Fixes: 658fa68b6f34 ("xfs: refactor log recovery inode item dispatch for pass2 commit functions")
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
fs/xfs/xfs_inode_item_recover.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
index 169a8fe3bf0a..388d3a8dbd31 100644
--- a/fs/xfs/xfs_inode_item_recover.c
+++ b/fs/xfs/xfs_inode_item_recover.c
@@ -510,6 +510,10 @@ xlog_recover_inode_commit_pass2(
switch (fields & XFS_ILOG_DFORK) {
case XFS_ILOG_DDATA:
case XFS_ILOG_DEXT:
+ if (len > XFS_DFORK_DSIZE(dip, mp)) {
+ error = -EFSCORRUPTED;
+ goto out_release;
+ }
memcpy(XFS_DFORK_DPTR(dip), src, len);
break;
@@ -545,8 +549,11 @@ xlog_recover_inode_commit_pass2(
switch (in_f->ilf_fields & XFS_ILOG_AFORK) {
case XFS_ILOG_ADATA:
case XFS_ILOG_AEXT:
+ if (len > XFS_DFORK_ASIZE(dip, mp)) {
+ error = -EFSCORRUPTED;
+ goto out_release;
+ }
dest = XFS_DFORK_APTR(dip);
- ASSERT(len <= XFS_DFORK_ASIZE(dip, mp));
memcpy(dest, src, len);
break;
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] xfs: bound inode fork length against the fork size during log recovery
2026-07-07 13:58 [PATCH] xfs: bound inode fork length against the fork size during log recovery Aldo Ariel Panzardo
@ 2026-07-07 16:26 ` Darrick J. Wong
2026-07-07 19:02 ` [PATCH v2] " Aldo Ariel Panzardo
1 sibling, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2026-07-07 16:26 UTC (permalink / raw)
To: Aldo Ariel Panzardo
Cc: linux-xfs, Carlos Maiolino, Chandan Babu R, linux-kernel
On Tue, Jul 07, 2026 at 10:58:43AM -0300, Aldo Ariel Panzardo wrote:
> xlog_recover_inode_commit_pass2() copies an inode log item's data/attr
> fork region into the inode buffer using the on-log region length without
> bounding it against the fork capacity, e.g.:
>
> len = item->ri_buf[2].iov_len;
> memcpy(XFS_DFORK_DPTR(dip), src, len);
>
> The only guard is an ASSERT, which is a no-op on production kernels
> (CONFIG_XFS_DEBUG off), and xfs_dinode_verify() runs only after the copy.
> A crafted image with a dirty log can therefore drive a heap out-of-bounds
> write at mount time. The XFS_ILOG_DBROOT sibling already passes
> XFS_DFORK_DSIZE as a bound; the DDATA/DEXT and ADATA/AEXT memcpy paths
> did not.
>
> Reject the log item with -EFSCORRUPTED when the region length exceeds the
> destination fork size.
>
> Fixes: 658fa68b6f34 ("xfs: refactor log recovery inode item dispatch for pass2 commit functions")
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
This is a bug fix, it should engage the LTS backporting process:
Cc: <stable@vger.kernel.org> # v5.8
> ---
> fs/xfs/xfs_inode_item_recover.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
> index 169a8fe3bf0a..388d3a8dbd31 100644
> --- a/fs/xfs/xfs_inode_item_recover.c
> +++ b/fs/xfs/xfs_inode_item_recover.c
> @@ -510,6 +510,10 @@ xlog_recover_inode_commit_pass2(
> switch (fields & XFS_ILOG_DFORK) {
> case XFS_ILOG_DDATA:
> case XFS_ILOG_DEXT:
> + if (len > XFS_DFORK_DSIZE(dip, mp)) {
> + error = -EFSCORRUPTED;
> + goto out_release;
> + }
But in any case this is new validation code, which means that it should
go at the top the function before we memcpy anything into the ondisk
inode.
--D
> memcpy(XFS_DFORK_DPTR(dip), src, len);
> break;
>
> @@ -545,8 +549,11 @@ xlog_recover_inode_commit_pass2(
> switch (in_f->ilf_fields & XFS_ILOG_AFORK) {
> case XFS_ILOG_ADATA:
> case XFS_ILOG_AEXT:
> + if (len > XFS_DFORK_ASIZE(dip, mp)) {
> + error = -EFSCORRUPTED;
> + goto out_release;
> + }
> dest = XFS_DFORK_APTR(dip);
> - ASSERT(len <= XFS_DFORK_ASIZE(dip, mp));
> memcpy(dest, src, len);
> break;
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v2] xfs: bound inode fork length against the fork size during log recovery
2026-07-07 13:58 [PATCH] xfs: bound inode fork length against the fork size during log recovery Aldo Ariel Panzardo
2026-07-07 16:26 ` Darrick J. Wong
@ 2026-07-07 19:02 ` Aldo Ariel Panzardo
1 sibling, 0 replies; 3+ messages in thread
From: Aldo Ariel Panzardo @ 2026-07-07 19:02 UTC (permalink / raw)
To: linux-xfs, Carlos Maiolino
Cc: Darrick J . Wong, Chandan Babu R, linux-kernel,
Aldo Ariel Panzardo, stable
xlog_recover_inode_commit_pass2() copies an inode log item's data/attr
fork region into the inode buffer using the on-log region length without
bounding it against the fork capacity, e.g.:
len = item->ri_buf[2].iov_len;
memcpy(XFS_DFORK_DPTR(dip), src, len);
The only guard is an ASSERT, which is a no-op on production kernels
(CONFIG_XFS_DEBUG off), and xfs_dinode_verify() runs only after the copy.
A crafted image with a dirty log can therefore drive a heap out-of-bounds
write at mount time. The XFS_ILOG_DBROOT sibling already passes
XFS_DFORK_DSIZE as a bound; the DDATA/DEXT and ADATA/AEXT memcpy paths
did not.
Bound each logged fork region against the destination fork size before
copying it, and reject the log item with -EFSCORRUPTED when it does not
fit. Because the recovered inode is only verified after the fork data has
been copied in, the checks are done up front, before any memcpy into the
on-disk inode.
Fixes: 658fa68b6f34 ("xfs: refactor log recovery inode item dispatch for pass2 commit functions")
Cc: <stable@vger.kernel.org> # v5.8
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v2: cc stable # v5.8 (per Darrick). Move both fork-length checks to the
top of the fork-copy block, before any memcpy into the on-disk
inode, and drop the now-redundant ASSERT.
fs/xfs/xfs_inode_item_recover.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
index 169a8fe3bf0a..6c7dd7dd7032 100644
--- a/fs/xfs/xfs_inode_item_recover.c
+++ b/fs/xfs/xfs_inode_item_recover.c
@@ -507,6 +507,25 @@ xlog_recover_inode_commit_pass2(
ASSERT(!(fields & XFS_ILOG_DFORK) ||
(len == xlog_calc_iovec_len(in_f->ilf_dsize)));
+ /*
+ * The recovered inode is verified only after the fork data has been
+ * copied into it, so bound each logged fork region against the size of
+ * its fork now, before the memcpy below can overrun the on-disk inode.
+ * The DBROOT/ABROOT cases already bound their copies against the fork
+ * size.
+ */
+ if ((fields & (XFS_ILOG_DDATA | XFS_ILOG_DEXT)) &&
+ item->ri_buf[2].iov_len > XFS_DFORK_DSIZE(dip, mp)) {
+ error = -EFSCORRUPTED;
+ goto out_release;
+ }
+ if ((fields & (XFS_ILOG_ADATA | XFS_ILOG_AEXT)) &&
+ item->ri_buf[(fields & XFS_ILOG_DFORK) ? 3 : 2].iov_len >
+ XFS_DFORK_ASIZE(dip, mp)) {
+ error = -EFSCORRUPTED;
+ goto out_release;
+ }
+
switch (fields & XFS_ILOG_DFORK) {
case XFS_ILOG_DDATA:
case XFS_ILOG_DEXT:
@@ -546,7 +565,6 @@ xlog_recover_inode_commit_pass2(
case XFS_ILOG_ADATA:
case XFS_ILOG_AEXT:
dest = XFS_DFORK_APTR(dip);
- ASSERT(len <= XFS_DFORK_ASIZE(dip, mp));
memcpy(dest, src, len);
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-07 19:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 13:58 [PATCH] xfs: bound inode fork length against the fork size during log recovery Aldo Ariel Panzardo
2026-07-07 16:26 ` Darrick J. Wong
2026-07-07 19:02 ` [PATCH v2] " Aldo Ariel Panzardo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox