Re: [PATCH] kernel/fork: stop playing lockless games for exe_file replacement

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14.08.23 17:58, Oleg Nesterov wrote:
On 08/14, David Hildenbrand wrote:

OK, I seem to understand... without mmap_read_lock() it is possible that

	- dup_mm_exe_file() sees mm->exe_file = old_exe_file

	- replace_mm_exe_file() does allow_write_access(old_exe_file)

	- another process does get_write_access(old_exe_file)

	- dup_mm_exe_file()->deny_write_access() fails

Right?

 From what I recall, yes.

Thanks! but then... David, this all is subjective, feel free to ignore, but
the current code doesn't look good to me, I mean the purpose of mmap_read_lock()
is very unclear. To me something like

	if (old_exe_file) {
		/*
		 * Ensure that if we race with dup_mm_exe_file() and it sees
		 * mm->exe_file == old_exe_file deny_write_access(old_exe_file)
		 * can't fail after we do allow_write_access() and another task
		 * does get_write_access(old_exe_file).
		 */
		mmap_read_lock(mm);
		mmap_read_unlock(mm);

		allow_write_access(old_exe_file);
		fput(old_exe_file);
	}

looks more understandable...

I don't particularly care about that code, and if there are ways to make it clearer, great.

As long as we can clarify in the patch description why we decided to go again the other direction (write lock) and not do what we did in 2015, that would be great.

--
Cheers,

David / dhildenb





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux