In the Linux kernel, the following vulnerability has been resolved:

xfs: fix under-reservation of blocks when repairing sf directories

Whilst running QA on XFS for-next as of 7.3-rc2 with MKFSOPTIONS="-n size=8192", I observed the following (trimmed) dmesg splat:

XFS: Assertion failed: args->total >= dp->inblocks - nblks, file: fs/xfs/libxfs/xfsdabtree.c, line: 2387 WARNING: fs/xfs/xfsmessage.c:104 at assfail+0x46/0x4a [xfs], CPU#0: xfsscrub/1426511 CPU: 0 UID: 0 PID: 1426511 Comm: xfsscrub Tainted: G W 7.3.0-rc2-djwx #rc2 PREEMPT(lazy) 6e418570b606a39783b0e7e7b30dc407b965f9e8 Tainted: [W]=WARN RIP: 0010:assfail+0x46/0x4a [xfs] RSP: 0018:ffffc900010d7890 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00000000ffffffd1 RDX: 0000000000000000 RSI: 0000000000000021 RDI: ffffffffa059fd38 RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000 R10: 000000000000000a R11: 000000007fffffff R12: ffffc900010d7940 R13: ffff888368d8f980 R14: ffffc900010d7a48 R15: ffffc900010d78d0 FS: 00007f445c5ce680(0000) GS:ffff8884a97ea000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f443803b9a8 CR3: 0000000107a4b000 CR4: 00000000003506f0 Call Trace: <TASK> xfsdagrowinodeint+0x2e0/0x300 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xfsdir2growinode+0x6e/0x150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xfsdir2sftoblock+0x149/0x870 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xrepdirswapprep+0xe2/0x110 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xrepdirswap+0xfb/0x2f0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xrepdirrebuildtree+0x99/0x100 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xrepdirectory+0x83/0x1c0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xrepattempt+0x4f/0x1e0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xfsscrubmetadata+0x393/0x5b0 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xfsiocscrubvmetadata+0x306/0x570 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] xfsfileioctl+0xa4f/0x1150 [xfs 5de2257e14108c136f11317e6bbb8ac77efd392c] x64sysioctl+0x76/0xc0 dosyscall64+0x7a/0x3b0 entrySYSCALL64afterhwframe+0x4b/0x53

This is a consequence of commit 0fe77e57588b98, which added the following assertion to xfsdagrowinodeint:

ASSERT(args->total >= dp->inblocks - nblks);

Tracing this back to xrepdirswapprep, I noticed that the xfsdaargs object that's passed to xfsdir2sftoblock sets args->total to 1. This is incorrect because mkfs set the directory block size to 8k and the filesystem block size to 4k. In other words, args->total should be 2 here, not 1.

Dave Chinner tripped over the same problem with the same branch through a different channel -- his test setup set the fs block size to 1k, in which case the directory block size is still set to 4k. Here, args->total should be 4.

Changing the assignment of args->total to sc->mp->mdirgeo->fsbcount makes the assertion go away, but that isn't a complete fix. In xreptempexchestimate, we also incorrectly assume that a shortform conversion requires 1 fsblock when it should be mdirgeo->fsbcount. Without that, we can under-reserve space in the transaction and cause a filesystem shutdown.

Note that the xfsdabufnfsb helper will compute the correct value for directories and xattr, so we use that instead of open-coding the logic. Also fix xrepxattrswapprep to assign args->total via xfsdabufnfsb to avoid one logic bomb if we ever support multi-fsblock attrs.

Tripped-by: 0fe77e57588b98 ("xfs: assert the reservation covers each da fork growth")

In the Linux kernel, the following vulnerability has been resolved:

xfs: delete attr leaf freemap entries when empty

Back in commit 2a2b5932db6758 ("xfs: fix attr leaf header freemap.size underflow"), Brian Foster observed that it's possible for a small freemap at the end of the end of the xattr entries array to experience a size underflow when subtracting the space consumed by an expansion of the entries array. There are only three freemap entries, which means that it is not a complete index of all free space in the leaf block.

This code can leave behind a zero-length freemap entry with a nonzero base. Subsequent setxattr operations can increase the base up to the point that it overlaps with another freemap entry. This isn't in and of itself a problem because the code in leafadd that finds free space ignores any freemap entry with zero size.

However, there's another bug in the freemap update code in leafadd, which is that it fails to update a freemap entry that begins midway through the xattr entry that was just appended to the array. That can result in the freemap containing two entries with the same base but different sizes (0 for the "pushed-up" entry, nonzero for the entry that's actually tracking free space). A subsequent leafadd can then allocate xattr namevalue entries on top of the entries array, leading to data loss. But fixing that is for later.

For now, eliminate the possibility of confusion by zeroing out the base of any freemap entry that has zero size. Because the freemap is not intended to be a complete index of free space, a subsequent failure to find any free space for a new xattr will trigger block compaction, which regenerates the freemap.

It looks like this bug has been in the codebase for quite a long time.

In the Linux kernel, the following vulnerability has been resolved:

xfs: fix freemap adjustments when adding xattrs to leaf blocks

xfs/592 and xfs/794 both trip this assertion in the leaf block freemap adjustment code after ~20 minutes of running on my test VMs:

ASSERT(ichdr->firstused >= ichdr->count sizeof(xfsattrleafentryt) + xfsattr3leafhdrsize(leaf));

Upon enabling quite a lot more debugging code, I narrowed this down to fsstress trying to set a local extended attribute with namelen=3 and valuelen=71. This results in an entry size of 80 bytes.

At the start of xfsattr3leafaddwork, the freemap looks like this:

i 0 base 448 size 0 rhs 448 count 46 i 1 base 388 size 132 rhs 448 count 46 i 2 base 2120 size 4 rhs 448 count 46 firstused = 520

where "rhs" is the first byte past the end of the leaf entry array. This is inconsistent -- the entries array ends at byte 448, but freemap[1] says there's free space starting at byte 388!

By the end of the function, the freemap is in worse shape:

i 0 base 456 size 0 rhs 456 count 47 i 1 base 388 size 52 rhs 456 count 47 i 2 base 2120 size 4 rhs 456 count 47 firstused = 440

Important note: 388 is not aligned with the entries array element size of 8 bytes.

Based on the incorrect freemap, the name area starts at byte 440, which is below the end of the entries array! That's why the assertion triggers and the filesystem shuts down.

How did we end up here? First, recall from the previous patch that the freemap array in an xattr leaf block is not intended to be a comprehensive map of all free space in the leaf block. In other words, it's perfectly legal to have a leaf block with:

376 bytes in use by the entries array freemap[0] has [base = 376, size = 8] freemap[1] has [base = 388, size = 1500] the space between 376 and 388 is free, but the freemap stopped tracking that some time ago

If we add one xattr, the entries array grows to 384 bytes, and freemap[0] becomes [base = 384, size = 0]. So far, so good. But if we add a second xattr, the entries array grows to 392 bytes, and freemap[0] gets pushed up to [base = 392, size = 0]. This is bad, because freemap[1] hasn't been updated, and now the entries array and the free space claim the same space.

The fix here is to adjust all freemap entries so that none of them collide with the entries array. Note that this fix relies on commit 2a2b5932db6758 ("xfs: fix attr leaf header freemap.size underflow") and the previous patch that resets zero length freemap entries to have base = 0.

In the Linux kernel, the following vulnerability has been resolved:

xfs: remove xfsattrleafhasname

The calling convention of xfsattrleafhasname() is problematic, because it returns a NULL buffer when xfsattr3leafread fails, a valid buffer when xfsattr3leaflookupint returns -ENOATTR or -EEXIST, and a non-NULL buffer pointer for an already released buffer when xfsattr3leaflookupint fails with other error values.

Fix this by simply open coding xfsattrleafhasname in the callers, so that the buffer release code is done by each caller of xfsattr3leafread.

In the Linux kernel, the following vulnerability has been resolved:

xfs: don't irele after failing to iget in xfsattrirecoverwork

xlogrecoveryiget never set @ip to a valid pointer if they return an error, so this irele will walk off a dangling pointer. Fix that.

In the Linux kernel, the following vulnerability has been resolved:

xfs: save ailp before dropping the AIL lock in push callbacks

In xfsinodeitempush() and xfsqmdquotlogitempush(), the AIL lock is dropped to perform buffer IO. Once the cluster buffer no longer protects the log item from reclaim, the log item may be freed by background reclaim or the dquot shrinker. The subsequent spinlock() call dereferences lip->liailp, which is a use-after-free.

Fix this by saving the ailp pointer in a local variable while the AIL lock is held and the log item is guaranteed to be valid.

In the Linux kernel, the following vulnerability has been resolved:

xfs: avoid dereferencing log items after push callbacks

After xfsaildpushitem() calls ioppush(), the log item may have been freed if the AIL lock was dropped during the push. Background inode reclaim or the dquot shrinker can free the log item while the AIL lock is not held, and the tracepoints in the switch statement dereference the log item after ioppush() returns.

Fix this by capturing the log item type, flags, and LSN before calling xfsaildpushitem(), and introducing a new xfsailpushclass trace event class that takes these pre-captured values and the ailp pointer instead of the log item pointer.