Re: Suggestion for clarifications on "man 5 proc" page regarding /proc/[pid]/self race conditions

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

Hi ellie,

On Tue, Dec 12, 2023 at 05:55:13PM +0100, ellie wrote:
> Thanks so much for the response!
> 
> For what it's worth, I checked my code again for the correct path, what I
> meant was /proc/self/exe which links to the binary of the currently running
> process, as far as I understand it.
> 
> I'm not sure it's easily possible to write a test program, because the
> open() wrapper by the libc on the /proc/self/exe symlink would need to be
> intercepted at just the right time in case /proc/self/exe is actually
> vulnerable. The breakpoint wouldn't be in the regular user code, might even
> be kernel code I guess, depending on where the race condition is located if
> it exists. (For FreeBSD a developer told me it supposedly exists for
> /proc/curproc/file which is apparently the equivalent, although that was
> about two years ago so I don't know if that has changed since.)

Do you suggest that open("/proc/self/exe", ...) could have a race within
the kernel?  It might be useful to CC <linux-kernel@vger.kernel.org> or
some other mailing list to be sure.

> 
> The wrong approach via readlink() on /proc/self/exe and then libc open() on
> the resulting path should be easy to intercept and break, but that doesn't
> really say much about the question at hand. I guess that this readlink
> approach isn't a good idea, even if commonly used, should be relatively
> obvious.

Now I'm not sure if the question at hand is that readlink(2)+open(2) has
a race (which of course it has), or if open("/proc/self/exe", ...) is
the race you're suggesting.  A patch for the former is welcome, and you
could add something like this to the commit message (or you could skip
it if you feel lazy, but these things help).

	int   fd, r;
	char  path[PATH_MAX];
	char  text[BUFSIZ];

	if (readlink("/proc/self/exe", path, NITEMS(path)) == -1)
		err(EXIT_FAILURE, "readlink");

	sleep(100);  // Give some time for the race
	// $ readlink /proc/$pid/exe \
	//   | while read f; do
	//           mv $f $f.bak;
	//           echo malicious >$f;
	//   done;

	fd = open(path, 0_RDONLY);
	if (fd == -1)
		err(EXIT_FAILURE, "open");

	r = read(fd, text, NITEMS(text));
	if (r == -1)
		err(EXIT_FAILURE, "read");
	if (write(STDOUT_FILENO, text, r) == -1)
		err(EXIT_FAILURE, "write");

Have a lovely day,
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]