lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2907412.VdNmn5OnKV@suse>
Date:   Wed, 08 Mar 2023 18:40:44 +0100
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Jan Kara <jack@...e.cz>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [git pull] vfs.git sysv pile

On giovedì 2 marzo 2023 20:35:59 CET Al Viro wrote:

[...]

> Frankly, ext2 patchset had been more along the lines of "here's what
> untangling the calling conventions in ext2 would probably look like" than
> anything else. If you are willing to test (and review) that sucker and it
> turns out to be OK, I'll be happy to slap your tested-by on those during
> rebase and feed them to Jan...

I git-clone(d) and built your "vfs" tree, branch #work.ext2, without and with 
the following commits:

f5b399373756 ("ext2: use offset_in_page() instead of open-coding it as 
subtraction")

c7248e221fb5 ("ext2_get_page(): saner type")

470e54a09898 ("ext2_put_page(): accept any pointer within the page")

15abcc147cf7 ("ext2_{set_link,delete_entry}(): don't bother with page_addr")

16a5ee2027b7 ("ext2_find_entry()/ext2_dotdot(): callers don't need page_addr 
anymore")

Then I read the code and FWIW the five patches look good to me. I think they 
can work properly. 

Therefore, if you want to, please feel free to add my "Reviewed-by" tag (OK, I 
know that you don't need my reviews, since you are the one who taught me how 
to write patches like yours for sysv and ufs :-)).

As a personal preference, in ext2_get_page() I'd move the two lines of code 
from the "fail" label to the same 'if' block where you have the "goto fail;", 
mainly because that label is only reachable from there. However, it does not 
matter at all because I'm only expressing my personal preference.

I ran `./check -g quick` without your patches in a QEMU/KVM x86_32 VM, 6GB 
RAM, running a Kernel with HIGHMEM64GB enabled. I ran it three or four times 
because it kept on hanging at random tests' numbers.

I'm noticing the same pattern due to the oom killer kicking in several times 
to kill processes until xfstests its is dead.

[ 1171.795551] Out of memory: Killed process 1669 (xdg-desktop-por) total-vm:
105068kB, anon-rss:9792kB, file-rss:10972kB, shmem-rss:0kB, UID:1000 pgtables:
136kB oom_score_adj:200
[ 1172.339920] systemd invoked oom-killer: gfp_mask=0xcc0(GFP_KERNEL), 
order=0, oom_score_adj=100
[ 1172.339927] CPU: 3 PID: 1413 Comm: systemd Tainted: G S      W   E      
6.3.0-rc1-x86-32-debug+ #1
[ 1172.339929] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
rel-1.16.0-0-gd239552-rebuilt.opensuse.org 04/01/2014
[ 1172.339931] Call Trace:
[ 1172.339934]  dump_stack_lvl+0x92/0xd4
[ 1172.339939]  dump_stack+0xd/0x10
[ 1172.339941]  dump_header+0x42/0x454
[ 1172.339945]  ? ___ratelimit+0x6f/0x140
[ 1172.339948]  oom_kill_process+0xe9/0x244
[ 1172.339950]  out_of_memory+0xf6/0x424 

I have not enough experience to understand why we get to that out-of-memory 
condition, so that several processes get killed. I can send the whole decoded 
stack trace and other information to whoever can look at this issue to figure 
out how to fix this big issue. I can try to bisect this issue too, but I need 
time because of other commitments and a slow system for building the necessary 
kernels.

I want to stress that it does not depend on the above-mentioned patches. Yes, 
I'm running Al's "vfs" tree, #work.ext2 branch, but with one only patch beyond 
the merge with Linus' tree:

522dad1 ext2_rename(): set_link and delete_entry may fail 

I have no means to test this tree. However, I think that I'd have the same 
issue with Linus' tree too, unless this issue is due to the only commit not 
yet there (I strongly doubt about this possibility).

Thanks,

Fabio





Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ