On Fri, Feb 10, 2023 at 11:51:03AM -0800, Leah Rumancik wrote: > On Wed, Feb 08, 2023 at 11:40:59AM -0800, Darrick J. Wong wrote: > > On Wed, Feb 08, 2023 at 09:02:58PM +0200, Amir Goldstein wrote: > > > On Wed, Feb 8, 2023 at 7:52 PM Leah Rumancik <leah.rumancik@xxxxxxxxx> wrote: > > > > > > > > Hello again, > > > > > > > > Here is the next batch of backports for 5.15.y. Testing included > > > > 25 runs of auto group on 12 xfs configs. No regressions were seen. > > > > I checked xfs/538 was run without issue as this test was mentioned > > > > in 56486f307100. Also, from 86d40f1e49e9, I ran ran xfs/117 with > > > > XFS compiled as a module and TEST_FS_MODULE_REOLOAD set, but I was > > > > unable to reproduce the issue. > > > > > > Did you find any tests that started to pass or whose failure rate reduced? > > > > I wish Leah had, but there basically aren't any tests for the problems > > fixed in this set for her to find. :( > > > > The first two patches I think were from when Dave was working on log > > intent whiteouts, turned on KASAN to diagnose some other problem he had, > > and began pulling on the ball of string (as it were) as he noticed other > > things in the codebase. We don't usually bother with regression tests > > for kernel memory leaks, since they're not so easy to reproduce. > > > > Patches 3-6 are fixes for a rash of fuzzer reports that someone in China > > posted last May: > > https://bugzilla.kernel.org/show_bug.cgi?id=215927 > > > > (There are more than just that one) > > > > As usual, the submitter didn't bother to help triage and just dumped a > > ton of work in our laps. They didn't follow up with any regression > > tests, because few fuzz kiddiez ever do. At the time, I was too burned > > out to deal with it, so Dave posted fixes. > > > > Patches 7-8 would manifest themselves as test VMs halting on ASSERTs if > > you configure your kernel to panic. Not strictly needed since most LTS > > kernels probably don't even have XFS_DEBUG=y, but it makes the lives of > > recoveryloop runners easier if they do. > > > > Patch 9 trips xfs/434 and xfs/436, but they only run if you have slab > > debugging enabled and build XFS as a module. I don't know why very few > > people do this. > > > > Patch 10 is a memory leak if you have XFS_DEBUG=y. No need for a > > separate test for this one, since kmemleak catches it. If you turn it > > on. > > > > (IOWs, LGTM for the whole set.) > > Good to add Ack tag? Oh, yes, sorry, I forgot about the formal tagging bit: Acked-by: Darrick J. Wong <djwong@xxxxxxxxxx> --D > > > > > > > > Leah, please consider working on the SGID bug fixes for the next 5.15 > > > update, because my 5.10 SGID fixes series [1] has been blocked for > > > months and because there are several reproducible test cases in xfstest. > > > > Whenever y'all get to 6.0, beware of commit 2ed5b09b3e8f ("xfs: make > > inode attribute forks a permanent part of struct xfs_inode"), which is a > > KASAN UAF that we fixed by eliminating the 'F'. Do not pull on the > > devilstring ("...the file capabilities code calls getxattr with and > > without i_rwsem held...") if you can avoid it. > > > > > I did not push on that until now because SGID test expectations were > > > a moving target, but since xfstests commit 81e6f628 ("generic: update > > > setgid tests") in this week's xfstests release, I think that tests should be > > > stable and we can finally start backporting all relevant SGID fixes to > > > align the SGID behavior of LTS kernels with that of upstream. > > Ooo goody, ok, will do this next. > > The following patches are on my radar to look into for this set. I have > yet to look into dependencies, so the set may grow. If the sgid tests > still fail after these ptaches, I will continue hunting for more fixes > to include in this set. > > e014f37db1a2 xfs: use setattr_copy to set vfs inode attributes > 472c6e46f589 xfs: remove XFS_PREALLOC_SYNC > fbe7e5200365 xfs: fallocate() should call file_modified() > 0b02c8c0d75a xfs: set prealloc flag in xfs_alloc_file_space() > 2b3416ceff5e fs: add mode_strip_sgid() helper > 1639a49ccdce fs: move S_ISGID stripping into the vfs_*() helpers > ed5a7047d201 attr: use consistent sgid stripping checks > 8d84e39d76bd fs: use consistent setgid checks in is_sxid() > > In addition to the normal regression testing, I will specifically look > at the following tests for the sgid changes: > > generic/673 > generic/68[3-7] > generic/69[6-7] > > I will also do some extra runs on the entire perms group. > > Let me know if you think something should be dropped or added. > > - Leah > > > > > Oh good, I've been (gently) waiting on that one too. :) > > > > --D > > > > > Thanks, > > > Amir. > > > > > > [1] https://github.com/amir73il/linux/commits/xfs-5.10.y-sgid-fixes > > > > > > > > > > > Below I've outlined which series the backports came from: > > > > > > > > series "xfs: intent whiteouts" (1): > > > > [01/10] cb512c921639613ce03f87e62c5e93ed9fe8c84d > > > > xfs: zero inode fork buffer at allocation > > > > [02/10] c230a4a85bcdbfc1a7415deec6caf04e8fca1301 > > > > xfs: fix potential log item leak > > > > > > > > series "xfs: fix random format verification issues" (2): > > > > [1/4] dc04db2aa7c9307e740d6d0e173085301c173b1a > > > > xfs: detect self referencing btree sibling pointers > > > > [2/4] 1eb70f54c445fcbb25817841e774adb3d912f3e8 -> already in 5.15.y > > > > xfs: validate inode fork size against fork format > > > > [3/4] dd0d2f9755191690541b09e6385d0f8cd8bc9d8f > > > > xfs: set XFS_FEAT_NLINK correctly > > > > [4/4] f0f5f658065a5af09126ec892e4c383540a1c77f > > > > xfs: validate v5 feature fields > > > > > > > > series "xfs: small fixes for 5.19 cycle" (3): > > > > [1/3] 5672225e8f2a872a22b0cecedba7a6644af1fb84 > > > > xfs: avoid unnecessary runtime sibling pointer endian conversions > > > > [2/3] 5b55cbc2d72632e874e50d2e36bce608e55aaaea > > > > fs: don't assert fail on perag references on teardown > > > > [2/3] 56486f307100e8fc66efa2ebd8a71941fa10bf6f > > > > xfs: assert in xfs_btree_del_cursor should take into account error > > > > > > > > series "xfs: random fixes for 5.19" (4): > > > > [1/2] 86d40f1e49e9a909d25c35ba01bea80dbcd758cb > > > > xfs: purge dquots after inode walk fails during quotacheck > > > > [2/2] a54f78def73d847cb060b18c4e4a3d1d26c9ca6d > > > > xfs: don't leak btree cursor when insrec fails after a split > > > > > > > > (1) https://lore.kernel.org/all/20220503221728.185449-1-david@xxxxxxxxxxxxx/ > > > > (2) https://lore.kernel.org/all/20220502082018.1076561-1-david@xxxxxxxxxxxxx/ > > > > (3) https://lore.kernel.org/all/20220524022158.1849458-1-david@xxxxxxxxxxxxx/ > > > > (4) https://lore.kernel.org/all/165337056527.993079.1232300816023906959.stgit@magnolia/ > > > > > > > > Darrick J. Wong (2): > > > > xfs: purge dquots after inode walk fails during quotacheck > > > > xfs: don't leak btree cursor when insrec fails after a split > > > > > > > > Dave Chinner (8): > > > > xfs: zero inode fork buffer at allocation > > > > xfs: fix potential log item leak > > > > xfs: detect self referencing btree sibling pointers > > > > xfs: set XFS_FEAT_NLINK correctly > > > > xfs: validate v5 feature fields > > > > xfs: avoid unnecessary runtime sibling pointer endian conversions > > > > xfs: don't assert fail on perag references on teardown > > > > xfs: assert in xfs_btree_del_cursor should take into account error > > > > > > > > fs/xfs/libxfs/xfs_ag.c | 3 +- > > > > fs/xfs/libxfs/xfs_btree.c | 175 +++++++++++++++++++++++++-------- > > > > fs/xfs/libxfs/xfs_inode_fork.c | 12 ++- > > > > fs/xfs/libxfs/xfs_sb.c | 70 +++++++++++-- > > > > fs/xfs/xfs_bmap_item.c | 2 + > > > > fs/xfs/xfs_icreate_item.c | 1 + > > > > fs/xfs/xfs_qm.c | 9 +- > > > > fs/xfs/xfs_refcount_item.c | 2 + > > > > fs/xfs/xfs_rmap_item.c | 2 + > > > > 9 files changed, 221 insertions(+), 55 deletions(-) > > > > > > > > -- > > > > 2.39.1.519.gcb327c4b5f-goog > > > >