| Message ID | 20210330104613.1059-1-zhanglianjie@uniontech.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | Modify the test logic of mincore. | expand |
Hi! > Currently mincore has a vulnerability and is easy to be attacked. > CVE has fixed the vulnerability. > Please see https://www.linuxkernelcves.com/cves/CVE-2019-5489 > > Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com> > --- > testcases/kernel/syscalls/mincore/mincore04.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/testcases/kernel/syscalls/mincore/mincore04.c b/testcases/kernel/syscalls/mincore/mincore04.c > index ed0ab7dfa..345dedd9a 100644 > --- a/testcases/kernel/syscalls/mincore/mincore04.c > +++ b/testcases/kernel/syscalls/mincore/mincore04.c > @@ -95,12 +95,12 @@ static void test_mincore(void) > locked_pages = count_pages_in_cache(); > tst_reap_children(); > > - if (locked_pages == NUM_PAGES) > - tst_res(TPASS, "mincore reports all %d pages locked by child process " > - "are resident", locked_pages); > - else > - tst_res(TFAIL, "mincore reports %d pages resident but %d pages " > + if (locked_pages == 0) > + tst_res(TPASS, "mincore reports %d pages resident but %d pages " > "locked by child process", locked_pages, NUM_PAGES); > + else > + tst_res(TFAIL, "mincore reports all %d pages locked by child process " > + "are resident", locked_pages); > } This does not make any sense, the kernel commit explicitly states that all mapped pages are reported as in core. We do call mlock() in the child, which will fault all the pages and lock them in memory. So the test should work both before and after the fix as well. The kernel commit in question weakened mincore() in a sense that it's more likely to report pages in core than it previously was. Now all that is needed is to fault the pages by reading some bytes from them to make sure they are reported as in core. If the test fails for you, something is probably broken at your end.
Hi, >> Currently mincore has a vulnerability and is easy to be attacked. >> CVE has fixed the vulnerability. >> Please see https://www.linuxkernelcves.com/cves/CVE-2019-5489 >> >> Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com> >> --- >> testcases/kernel/syscalls/mincore/mincore04.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/mincore/mincore04.c b/testcases/kernel/syscalls/mincore/mincore04.c >> index ed0ab7dfa..345dedd9a 100644 >> --- a/testcases/kernel/syscalls/mincore/mincore04.c >> +++ b/testcases/kernel/syscalls/mincore/mincore04.c >> @@ -95,12 +95,12 @@ static void test_mincore(void) >> locked_pages = count_pages_in_cache(); >> tst_reap_children(); >> >> - if (locked_pages == NUM_PAGES) >> - tst_res(TPASS, "mincore reports all %d pages locked by child process " >> - "are resident", locked_pages); >> - else >> - tst_res(TFAIL, "mincore reports %d pages resident but %d pages " >> + if (locked_pages == 0) >> + tst_res(TPASS, "mincore reports %d pages resident but %d pages " >> "locked by child process", locked_pages, NUM_PAGES); >> + else >> + tst_res(TFAIL, "mincore reports all %d pages locked by child process " >> + "are resident", locked_pages); >> } > > This does not make any sense, the kernel commit explicitly states that > all mapped pages are reported as in core. We do call mlock() in the > child, which will fault all the pages and lock them in memory. So the > test should work both before and after the fix as well. > > The kernel commit in question weakened mincore() in a sense that it's > more likely to report pages in core than it previously was. Now all that > is needed is to fault the pages by reading some bytes from them to make > sure they are reported as in core. > > If the test fails for you, something is probably broken at your end. > Using the CVE patch on mips, the test will fail because the child process mlocks all pages, but the parent process cannot obtain these pages through mincore and is locked.
Hi! > > This does not make any sense, the kernel commit explicitly states that > > all mapped pages are reported as in core. We do call mlock() in the > > child, which will fault all the pages and lock them in memory. So the > > test should work both before and after the fix as well. > > > > The kernel commit in question weakened mincore() in a sense that it's > > more likely to report pages in core than it previously was. Now all that > > is needed is to fault the pages by reading some bytes from them to make > > sure they are reported as in core. > > > > If the test fails for you, something is probably broken at your end. > > > > Using the CVE patch on mips, the test will fail because the child > process mlocks all pages, but the parent process cannot obtain these > pages through mincore and is locked. Sounds like a kernel bug.
Hi! >>> This does not make any sense, the kernel commit explicitly states that >>> all mapped pages are reported as in core. We do call mlock() in the >>> child, which will fault all the pages and lock them in memory. So the >>> test should work both before and after the fix as well. >>> >>> The kernel commit in question weakened mincore() in a sense that it's >>> more likely to report pages in core than it previously was. Now all that >>> is needed is to fault the pages by reading some bytes from them to make >>> sure they are reported as in core. >>> >>> If the test fails for you, something is probably broken at your end. >>> >> >> Using the CVE patch on mips, the test will fail because the child >> process mlocks all pages, but the parent process cannot obtain these >> pages through mincore and is locked. > > Sounds like a kernel bug. > CVE list has provided repair patches, The patch I submitted this time is to modify the logic of the test results, do you plan to incorporate it?
Hi! > >> Using the CVE patch on mips, the test will fail because the child > >> process mlocks all pages, but the parent process cannot obtain these > >> pages through mincore and is locked. > > > > Sounds like a kernel bug. > > > CVE list has provided repair patches, The patch I submitted this time is > to modify the logic of the test results, do you plan to incorporate it? Let me try to explain it once again. As far as I can tell the CVE kernel patch you pointed out has NO effect on the testcase and there is NO need to modify the test at all. The test works fine both before and after the kernel patch. If the test fails for you YOUR kernel is broken. That is unless you prove that the test is actually wrong, which you haven't and I do not think that you actually can.
Hi, > Hi! >>>> Using the CVE patch on mips, the test will fail because the child >>>> process mlocks all pages, but the parent process cannot obtain these >>>> pages through mincore and is locked. >>> >>> Sounds like a kernel bug. >>> >> CVE list has provided repair patches, The patch I submitted this time is >> to modify the logic of the test results, do you plan to incorporate it? > > Let me try to explain it once again. > > As far as I can tell the CVE kernel patch you pointed out has NO effect > on the testcase and there is NO need to modify the test at all. The test > works fine both before and after the kernel patch. > > If the test fails for you YOUR kernel is broken. > > That is unless you prove that the test is actually wrong, which you > haven't and I do not think that you actually can. > The test was passed before the cve patch was applied, and the test failed after the patch was applied, and the test results were the same on both the x86 and mips architectures. This cve patch changes mincore() to count "mapped" pages instead of "cached" pages.
Hi! > The test was passed before the cve patch was applied, and the test > failed after the patch was applied, and the test results were the same > on both the x86 and mips architectures. > > This cve patch changes mincore() to count "mapped" pages instead of > "cached" pages. I've looked at at the mincore code and found what is your problem. The initiall patch that fixed the CVE was wrong and later reverted and replaced with a different fix: commit 134fca9063ad4851de767d1768180e5dede9a881 Author: Jiri Kosina <jkosina@suse.cz> Date: Tue May 14 15:41:38 2019 -0700 mm/mincore.c: make mincore() more conservative So it's your kernel that is broken and the test is fine.
diff --git a/testcases/kernel/syscalls/mincore/mincore04.c b/testcases/kernel/syscalls/mincore/mincore04.c index ed0ab7dfa..345dedd9a 100644 --- a/testcases/kernel/syscalls/mincore/mincore04.c +++ b/testcases/kernel/syscalls/mincore/mincore04.c @@ -95,12 +95,12 @@ static void test_mincore(void) locked_pages = count_pages_in_cache(); tst_reap_children(); - if (locked_pages == NUM_PAGES) - tst_res(TPASS, "mincore reports all %d pages locked by child process " - "are resident", locked_pages); - else - tst_res(TFAIL, "mincore reports %d pages resident but %d pages " + if (locked_pages == 0) + tst_res(TPASS, "mincore reports %d pages resident but %d pages " "locked by child process", locked_pages, NUM_PAGES); + else + tst_res(TFAIL, "mincore reports all %d pages locked by child process " + "are resident", locked_pages); } static struct tst_test test = { @@ -109,4 +109,9 @@ static struct tst_test test = { .forks_child = 1, .test_all = test_mincore, .needs_checkpoints = 1, + .tags = (const struct tst_tag[]) { + {"linux-git", "574823bfab82"}, + {"CVE", "2019-5489"}, + {} + } };
Currently mincore has a vulnerability and is easy to be attacked. CVE has fixed the vulnerability. Please see https://www.linuxkernelcves.com/cves/CVE-2019-5489 Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com> --- testcases/kernel/syscalls/mincore/mincore04.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) -- 2.20.1