[LTP] [PATCH v1] security/dirtyc0w_shmem: Add new test for CVE-2022-2590

[David Hildenbrand]

On 21.11.22 15:59, David Hildenbrand wrote:
> On 18.11.22 14:29, Cyril Hrubis wrote:
>> Hi!
> 
> Hi Cyril,
> 
> thanks for your review!
> 
> [...]
> 
>>> +
>>> +/*
>>> + * CVE-2022-2590
>>> + *
>>> + * This is a regression test for a write race that allows unprivileged programs
>>> + * to change readonly files located on tmpfs/shmem on the system.
>>> + *
>>> + * Fixed by:
>>> + *
>>> + *   commit 5535be3099717646781ce1540cf725965d680e7b
>>> + *   Author: David Hildenbrand <david@redhat.com>
>>> + *   Date:   Tue Aug 9 22:56:40 2022 +0200
>>> + *
>>> + *   mm/gup: fix FOLL_FORCE COW security issue and remove FOLL_COW
>>> + */
>>
>> This is not a proper documentation comment. We do have a system in place
>> that picks up ascii-doc formatted documentation comments and builds a
>> documentation based on that.
>>
>> The documentation comment has to start with:
>>
>> /*\
>>    * [Description]
>>    *
>>
>>
>> Also the CVE and kernel commit are picked from tags when documentation
>> is build, so they do not need to be part of the documentation comment.
>>
> 
> I used dirtyc0w.c as orientation, which probably wasn't a good idea.
> 
> I'll use a simple
> 
> /*\
>    * [Description]
>    *
>    * This is a regression test for a write race that allowed unprivileged programs
>    * to change readonly files located on tmpfs/shmem on the system using
>    * userfaultfd "minor fault handling" (CVE-2022-2590).
>    */
> 
>>> +#include "config.h"
>>> +
>>> +#include <pthread.h>
>>> +#include <unistd.h>
>>> +#include <sys/stat.h>
>>> +#include <string.h>
>>> +#include <stdlib.h>
>>> +#include <stdbool.h>
>>> +#include <pwd.h>
>>> +
>>> +#include "tst_test.h"
>>> +
>>> +#define TMP_DIR "tmp_dirtyc0w_shmem"
>>> +#define TEST_FILE TMP_DIR"/testfile"
>>> +#define STR "this is not a test\n"
>>> +
>>> +static uid_t nobody_uid;
>>> +static gid_t nobody_gid;
>>> +static bool child_early_exit;
>>
>> Anything that is changed from signal handler should be volatile
>> otherwise it may end up optimized out.
> 
> Ack.
> 
>>
>>> +static void sighandler(int sig)
>>> +{
>>> +	if (sig == SIGCHLD) {
>>> +		child_early_exit = true;
>>> +		return;
>>> +	}
>>> +
>>> +	_exit(0);
>>> +}
>>> +
>>> +static void setup(void)
>>> +{
>>> +	struct passwd *pw;
>>> +
>>> +	umask(0);
>>> +
>>> +	pw = SAFE_GETPWNAM("nobody");
>>> +
>>> +	nobody_uid = pw->pw_uid;
>>> +	nobody_gid = pw->pw_gid;
>>> +
>>> +	SAFE_MKDIR(TMP_DIR, 0664);
>>> +	SAFE_MOUNT(TMP_DIR, TMP_DIR, "tmpfs", 0, NULL);
>>> +}
>>> +
>>> +static void dirtyc0w_shmem_test(void)
>>> +{
>>> +	bool failed = false;
>>> +	int fd, pid;
>>> +	char c;
>>> +
>>> +	fd = SAFE_OPEN(TEST_FILE, O_WRONLY|O_CREAT|O_EXCL, 0444);
>>> +	SAFE_WRITE(SAFE_WRITE_ALL, fd, STR, sizeof(STR)-1);
>>> +	SAFE_CLOSE(fd);
>>
>> SAFE_FILE_PRINTF() ?
> 
> AFAIU, that would mean
> 
> SAFE_FILE_PRINTF(TEST_FILE, "%s", STR);

SAFE_FILE_PRINTF(TEST_FILE, STR);

to be precise.

Seems to get the job done.

-- 
Thanks,

David / dhildenb