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

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

[ellie]

Dear Alejandro Colomar,

I hope I'm emailing this to the correct place, I found this contact 
information on https://man7.org/mtk/contact.html regarding man page 
feedback:

I'm suggesting that the "man 5 proc" page is expanded with a section 
clarifying /proc/[pid]/self race conditions, I described details and 
even made a text suggestion here:

https://bugzilla.suse.com/show_bug.cgi?id=1216352

(The text suggestion might be wrong, however, since I don't actually 
know what the exact technical state of this is.)

Regards,

ellie

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

[Alejandro Colomar]

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

Dear ellie, Petr,

On Tue, Dec 12, 2023 at 09:47:58AM +0100, ellie wrote:
> Dear Alejandro Colomar,
> 
> I hope I'm emailing this to the correct place, I found this contact
> information on https://man7.org/mtk/contact.html regarding man page
> feedback:

Yep, this is the correct place.

Petr, Michael retired from maintaining the project a couple of years
ago.  As Michael says in <https://www.man7.org/mtk/contact.html>,
reports about the Linux man-pages should be reported to this mailing
list, following the ./CONTRIBUTING file
<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING>.

> 
> I'm suggesting that the "man 5 proc" page is expanded with a section

I've recently splitted the proc(5) page into many small pages, one for
each file or directory.  You may want to check the current manual pages.
You can do that by reading directly from the repository, or by reading
the PDF book (thanks to Deri James, from gropdf(1), for contributing the
scripts to produce the book).

To check the book as of the latest commit in git HEAD, you can check
<https://www.alejandro-colomar.es/share/dist/man-pages/git/HEAD/man-pages-HEAD.pdf>

You will probably want to check proc_self(5) --which is a link page; the
text is actually in proc_pid(5)--.  proc(5) still contains some small
mention of /proc/self, so you'll want to check that too.

> clarifying /proc/[pid]/self

You probably mean /proc/self/, or /proc/[pid]/.

> race conditions, I described details and even
> made a text suggestion here:
> 
> https://bugzilla.suse.com/show_bug.cgi?id=1216352

After seeing the suggestion, you'll want to check proc_pid_exe(5):

<https://www.alejandro-colomar.es/share/dist/man-pages/git/HEAD/man-pages-HEAD.pdf#proc_pid_exe.5>

You could add a CAVEATS section in that page.  Please write also a test
program that reproduces the race condition, and another one which
demonstrates how your solution doesn't.  Those test programs will be
useful to include in the commit message.

> (The text suggestion might be wrong, however, since I don't actually know
> what the exact technical state of this is.)

It looks good.  With a small example program that demonstrates it,
you'll be able to answer your doubts.  ;-)

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 --]

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

[ellie]

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.)

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.

Regards,

ellie

On 12/12/23 3:17 PM, Alejandro Colomar wrote:
> Dear ellie, Petr,
> 
> On Tue, Dec 12, 2023 at 09:47:58AM +0100, ellie wrote:
>> Dear Alejandro Colomar,
>>
>> I hope I'm emailing this to the correct place, I found this contact
>> information on https://man7.org/mtk/contact.html regarding man page
>> feedback:
> 
> Yep, this is the correct place.
> 
> Petr, Michael retired from maintaining the project a couple of years
> ago.  As Michael says in <https://www.man7.org/mtk/contact.html>,
> reports about the Linux man-pages should be reported to this mailing
> list, following the ./CONTRIBUTING file
> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING>.
> 
>>
>> I'm suggesting that the "man 5 proc" page is expanded with a section
> 
> I've recently splitted the proc(5) page into many small pages, one for
> each file or directory.  You may want to check the current manual pages.
> You can do that by reading directly from the repository, or by reading
> the PDF book (thanks to Deri James, from gropdf(1), for contributing the
> scripts to produce the book).
> 
> To check the book as of the latest commit in git HEAD, you can check
> <https://www.alejandro-colomar.es/share/dist/man-pages/git/HEAD/man-pages-HEAD.pdf>
> 
> You will probably want to check proc_self(5) --which is a link page; the
> text is actually in proc_pid(5)--.  proc(5) still contains some small
> mention of /proc/self, so you'll want to check that too.
> 
>> clarifying /proc/[pid]/self
> 
> You probably mean /proc/self/, or /proc/[pid]/.
> 
>> race conditions, I described details and even
>> made a text suggestion here:
>>
>> https://bugzilla.suse.com/show_bug.cgi?id=1216352
> 
> After seeing the suggestion, you'll want to check proc_pid_exe(5):
> 
> <https://www.alejandro-colomar.es/share/dist/man-pages/git/HEAD/man-pages-HEAD.pdf#proc_pid_exe.5>
> 
> You could add a CAVEATS section in that page.  Please write also a test
> program that reproduces the race condition, and another one which
> demonstrates how your solution doesn't.  Those test programs will be
> useful to include in the commit message.
> 
>> (The text suggestion might be wrong, however, since I don't actually know
>> what the exact technical state of this is.)
> 
> It looks good.  With a small example program that demonstrates it,
> you'll be able to answer your doubts.  ;-)
> 
> Have a lovely day!
> Alex
> 

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

[Alejandro Colomar]

[-- 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 --]

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

[ellie]

Hi Alejandro,

Thanks so much for elaborating! Just to clear it up, I hope it's obvious 
enough readlink("/proc/self/exe") to get a path, and then using open() 
on the result has a race by design: of the binary targeted by that path 
being renamed and replaced with a different one between the readlink and 
the open. Yet this approach seems to be commonly used, so a warning on 
the man page listing /proc/self/exe not to do that seems useful.

Now whether open("/proc/self/exe") has the same race condition would 
likely deoend on what that does behind the scenes. If /proc/self/exe is 
internally resolved to a path string first and not directly to an inode, 
then it should suffer the same race condition but with a smaller time 
window. If instead the kernel implementation is smart enough to not 
actually handle it like a "true" symlink, but rather look up the inode 
associated with the process directly, it would likely be safe.

Since I vaguely remember /proc/self/exe still working when the inode was 
deleted, I assume it's likely something smarter and might be 
race-condition safe when directly opened. But it would be nice to have 
some more definite info on that in the man page, just to be safe.

My apologies if I got some of the technical parts of this wrong, but 
hopefully it provides some explanation of why I brought this up.

Regards,

ellie

On 12/12/23 6:39 PM, Alejandro Colomar wrote:
> 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
> 

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

[Alejandro Colomar]

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

Hi ellie,

On Tue, Dec 12, 2023 at 06:55:22PM +0100, ellie wrote:
> Hi Alejandro,
> 
> Thanks so much for elaborating! Just to clear it up, I hope it's obvious
> enough readlink("/proc/self/exe") to get a path, and then using open() on
> the result has a race by design: of the binary targeted by that path being
> renamed and replaced with a different one between the readlink and the open.
> Yet this approach seems to be commonly used, so a warning on the man page
> listing /proc/self/exe not to do that seems useful.

Would you mind sending a patch for that?

> 
> Now whether open("/proc/self/exe") has the same race condition would likely
> deoend on what that does behind the scenes. If /proc/self/exe is internally
> resolved to a path string first and not directly to an inode, then it should
> suffer the same race condition but with a smaller time window. If instead
> the kernel implementation is smart enough to not actually handle it like a
> "true" symlink, but rather look up the inode associated with the process
> directly, it would likely be safe.
> 
> Since I vaguely remember /proc/self/exe still working when the inode was
> deleted, I assume it's likely something smarter and might be race-condition
> safe when directly opened. But it would be nice to have some more definite
> info on that in the man page, just to be safe.

I'm going to guess a little bit, from the following script:

	$ cat ppe.sh 
	#!/home/alx/tmp/bash

	f="$(readlink /proc/$$/exe)";

	set -x;

	mv $f $f.bak;
	cp /usr/bin/bash $f;
	readlink /proc/$$/exe;

	cp /usr/bin/bash $f.bak;
	readlink /proc/$$/exe;

	rm $f.bak;
	readlink /proc/$$/exe;

	echo malicious >$f.bak;
	readlink /proc/$$/exe;

	head -c4 /proc/$$/exe;


	$ ./ppe.sh 
	+ mv /home/alx/tmp/bash /home/alx/tmp/bash.bak
	+ cp /usr/bin/bash /home/alx/tmp/bash
	+ readlink /proc/5055/exe
	/home/alx/tmp/bash.bak
	+ cp /usr/bin/bash /home/alx/tmp/bash.bak
	cp: cannot create regular file '/home/alx/tmp/bash.bak': Text file busy
	+ readlink /proc/5055/exe
	/home/alx/tmp/bash.bak
	+ rm /home/alx/tmp/bash.bak
	+ readlink /proc/5055/exe
	/home/alx/tmp/bash.bak (deleted)
	+ echo malicious
	+ readlink /proc/5055/exe
	/home/alx/tmp/bash.bak (deleted)
	+ head -c4 /proc/5055/exe
	ELF

It doesn't seem to care about the text of the symlink, which in fact
changes as much as the file itself does.  There don't seem to be any
races in open("/proc/[pid]/exe", ...).

> 
> My apologies if I got some of the technical parts of this wrong, but
> hopefully it provides some explanation of why I brought this up.

Sure.

Have a nice 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 --]

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

[ellie]

I'll have to look over the rest later, but for what it's worth:

On 12/13/23 10:31 AM, Alejandro Colomar wrote:
> There don't seem to be any
> races in open("/proc/[pid]/exe", ...).

Sorry if I'm just confused, but I don't understand how the given test 
script reproduces or tests anything relevant to a potential open() race.

The race would work like this:

1. Process A issues open("/proc/self/exe")

2. Process A's open() on /proc/self/exe heads into whatever libc or 
kernel path that resolves where that symlink points to a path, if it's 
treated as one.

3. Process scheduler switches to process B.

4. Process B switches out process A's binary, such that a different 
binary is now at the old path.

5. Process A's open() code in libc or kernel space resumes and opens the 
file pointed to by the given path, which is now a new binary.

I'm pretty sure you can't test that anyway to rule it out, that could 
only be answered by looking at all the relevant code and whether 
/proc/self/exe is ever resolved to a path (like I assume an actual 
symlink usually is) or directly to an inode (which would likely be safe).

Regards,

ellie

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

[Alejandro Colomar]

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

Hi ellie,

On Wed, Dec 13, 2023 at 10:31:02AM +0100, Alejandro Colomar wrote:
> I'm going to guess a little bit, from the following script:
> 
> 	$ cat ppe.sh 
> 	#!/home/alx/tmp/bash
> 
> 	f="$(readlink /proc/$$/exe)";
> 
> 	set -x;
> 
> 	mv $f $f.bak;
> 	cp /usr/bin/bash $f;
> 	readlink /proc/$$/exe;
> 
> 	cp /usr/bin/bash $f.bak;
> 	readlink /proc/$$/exe;
> 
> 	rm $f.bak;
> 	readlink /proc/$$/exe;
> 
> 	echo malicious >$f.bak;
> 	readlink /proc/$$/exe;
> 
> 	head -c4 /proc/$$/exe;

On 12/13/23 10:31 AM, Alejandro Colomar wrote:
> There don't seem to be any
> races in open("/proc/[pid]/exe", ...).

ellie wrote:
| Sorry if I'm just confused, but I don't understand how the given test 
| script reproduces or tests anything relevant to a potential open() race.
| 
| The race would work like this:
| 
| 1. Process A issues open("/proc/self/exe")
| 
| 2. Process A's open() on /proc/self/exe heads into whatever libc or 
| kernel path that resolves where that symlink points to a path, if it's 
| treated as one.
| 
| 3. Process scheduler switches to process B.
| 
| 4. Process B switches out process A's binary, such that a different 
| binary is now at the old path.
| 
| 5. Process A's open() code in libc or kernel space resumes and opens the 
| file pointed to by the given path, which is now a new binary.
| 
| I'm pretty sure you can't test that anyway to rule it out, that could 
| only be answered by looking at all the relevant code and whether 
| /proc/self/exe is ever resolved to a path (like I assume an actual 
| symlink usually is) or directly to an inode (which would likely be safe).

I didn't test exactly that, but I proved that it doesn't work like a
symlink.  If it were a simple symlink, my script wouldn't get the new
path of the file after moving or removing it, and the symlink would
become dangling.

Still, the kernel could do the stupid thing: magically update the
symlink when the file changes, but still use a symlink, so open(2) would
still have a race.  I'm assuming the kernel doesn't do that stupid thing.

It must be implemented using the file description, or something like
that, and thus it would be free of races.

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 --]

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

[ellie]

Yes, that makes sense. I suspect that you are correct. However, it would 
be nice to have a definite confirmation by someone, hence also my 
suggestion to address it in the man page.

I can make and send a patch suggestion (I hope I'll figure out the 
correct format) in a few days with actual man page text changes under 
the assumption that open("/proc/self/exe") is free of races, but I'm 
probably not the person to actually verify that this assumption is correct.

On 12/13/23 2:54 PM, Alejandro Colomar wrote:
> Hi ellie,
> 
> On Wed, Dec 13, 2023 at 10:31:02AM +0100, Alejandro Colomar wrote:
>> I'm going to guess a little bit, from the following script:
>>
>> 	$ cat ppe.sh
>> 	#!/home/alx/tmp/bash
>>
>> 	f="$(readlink /proc/$$/exe)";
>>
>> 	set -x;
>>
>> 	mv $f $f.bak;
>> 	cp /usr/bin/bash $f;
>> 	readlink /proc/$$/exe;
>>
>> 	cp /usr/bin/bash $f.bak;
>> 	readlink /proc/$$/exe;
>>
>> 	rm $f.bak;
>> 	readlink /proc/$$/exe;
>>
>> 	echo malicious >$f.bak;
>> 	readlink /proc/$$/exe;
>>
>> 	head -c4 /proc/$$/exe;
> 
> On 12/13/23 10:31 AM, Alejandro Colomar wrote:
>> There don't seem to be any
>> races in open("/proc/[pid]/exe", ...).
> 
> ellie wrote:
> | Sorry if I'm just confused, but I don't understand how the given test
> | script reproduces or tests anything relevant to a potential open() race.
> |
> | The race would work like this:
> |
> | 1. Process A issues open("/proc/self/exe")
> |
> | 2. Process A's open() on /proc/self/exe heads into whatever libc or
> | kernel path that resolves where that symlink points to a path, if it's
> | treated as one.
> |
> | 3. Process scheduler switches to process B.
> |
> | 4. Process B switches out process A's binary, such that a different
> | binary is now at the old path.
> |
> | 5. Process A's open() code in libc or kernel space resumes and opens the
> | file pointed to by the given path, which is now a new binary.
> |
> | I'm pretty sure you can't test that anyway to rule it out, that could
> | only be answered by looking at all the relevant code and whether
> | /proc/self/exe is ever resolved to a path (like I assume an actual
> | symlink usually is) or directly to an inode (which would likely be safe).
> 
> I didn't test exactly that, but I proved that it doesn't work like a
> symlink.  If it were a simple symlink, my script wouldn't get the new
> path of the file after moving or removing it, and the symlink would
> become dangling.
> 
> Still, the kernel could do the stupid thing: magically update the
> symlink when the file changes, but still use a symlink, so open(2) would
> still have a race.  I'm assuming the kernel doesn't do that stupid thing.
> 
> It must be implemented using the file description, or something like
> that, and thus it would be free of races.
> 
> Have a lovely day,
> Alex
> 

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

[ellie]

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

Dear Alejandro Colomar,

I finally wrote a patch against the latest man pages, see file attached. 
  Hopefully this is in a format that allows you to integrate it well.

Some quick notes:

1. The "make" process aborted with an error, I couldn't use it to 
verify. However, pandoc seems to think my formatting is correct.

2. I still don't know if open("/proc/self/exe") has any internal race 
conditions on rename, like any more regular symlink. My text simply 
assumes it doesn't. Maybe some expert might want to double-check?

3. You can freely use my patch, no attribution needed. However, if 
source info is desired, you can supply "Ellie <el@horse64.org>" for that.

Regards,

Ellie

PS: The error that "make" gave me was this one, in case anybody finds 
this useful: "TROFF .tmp/man/man2/s390_sthyi.2.cat.set
troff:.tmp/man/man2/s390_sthyi.2:124: warning [p 2, 1.8i]: cannot adjust 
line"

On 12/12/23 9:47 AM, ellie wrote:
> Dear Alejandro Colomar,
> 
> I hope I'm emailing this to the correct place, I found this contact 
> information on https://man7.org/mtk/contact.html regarding man page 
> feedback:
> 
> I'm suggesting that the "man 5 proc" page is expanded with a section 
> clarifying /proc/[pid]/self race conditions, I described details and 
> even made a text suggestion here:
> 
> https://bugzilla.suse.com/show_bug.cgi?id=1216352
> 
> (The text suggestion might be wrong, however, since I don't actually 
> know what the exact technical state of this is.)
> 
> Regards,
> 
> ellie

[-- Attachment #2: man.patch --]
[-- Type: text/x-patch, Size: 1429 bytes --]

diff --git a/man/man5/proc.5 b/man/man5/proc.5
index d3bc28ff0..12d3d06b8 100644
--- a/man/man5/proc.5
+++ b/man/man5/proc.5
@@ -168,7 +168,10 @@ to view the contents of
 When a process accesses this magic symbolic link,
 it resolves to the process's own
 .IR /proc/ pid
-directory.
+directory. For notes on the thread-safety of
+.I /proc/self/exe,
+check
+.BR proc_pid_exe (5).
 .TP
 .I /proc/thread\-self
 When a thread accesses this magic symbolic link,
diff --git a/man/man5/proc_pid_exe.5 b/man/man5/proc_pid_exe.5
index e308677f1..aa8cddd70 100644
--- a/man/man5/proc_pid_exe.5
+++ b/man/man5/proc_pid_exe.5
@@ -55,5 +55,22 @@ MFM, etc. drives) minor 01 (first partition on the first drive).
 with the
 .I \-inum
 option can be used to locate the file.
+.SS Safely using proc/self/exe
+There is a common programming mistake of first using
+.IR readlink("/proc/self/exe")
+to obtain a program's own binary, and then using
+.IR open()
+on the resulting path string. In many scenarios this is unsafe, since
+between the two calls the binary may be renamed and then
+.IR open()
+would access an unrelated file.
+.P
+To avoid this pitfall, directly use
+.IR open("/proc/self/exe")
+without obtaining the filesystem path first. Since
+.IR /proc/self/exe
+is special and not a regular symlink, this opens the file directly
+without ever involving the filesystem path. This avoids any race
+conditions.
 .SH SEE ALSO
 .BR proc (5)

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

[Alejandro Colomar]

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

Hi Ellie,

On Sat, Jun 08, 2024 at 12:23:11AM GMT, ellie wrote:
> Dear Alejandro Colomar,
> 
> I finally wrote a patch against the latest man pages, see file attached.
> Hopefully this is in a format that allows you to integrate it well.
> 
> Some quick notes:
> 
> 1. The "make" process aborted with an error, I couldn't use it to verify.
> However, pandoc seems to think my formatting is correct.
> 
> 
> Regards,
> 
> Ellie
> 
> PS: The error that "make" gave me was this one, in case anybody finds this
> useful: "TROFF .tmp/man/man2/s390_sthyi.2.cat.set
> troff:.tmp/man/man2/s390_sthyi.2:124: warning [p 2, 1.8i]: cannot adjust
> line"

Huh, you're the second one to report that error, both in this weekend.

Can you please pass --debug=print to make to see more details?

Also, you can skip that target with

	$ touch .tmp/man/man2/s390_sthyi.2.cat.set

And run make(1) again.


Have a lovely day!
Alex


P.S.:  I'll check the patch later.

-- 
<https://www.alejandro-colomar.es/>

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

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

[Alejandro Colomar]

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

On Sat, Jun 08, 2024 at 12:23:11AM GMT, ellie wrote:
> Dear Alejandro Colomar,
> 
> I finally wrote a patch against the latest man pages, see file attached.
> Hopefully this is in a format that allows you to integrate it well.
> 
> Some quick notes:
> 
> 1. The "make" process aborted with an error, I couldn't use it to verify.
> However, pandoc seems to think my formatting is correct.
> 
> 2. I still don't know if open("/proc/self/exe") has any internal race
> conditions on rename, like any more regular symlink. My text simply assumes
> it doesn't. Maybe some expert might want to double-check?
> 
> 3. You can freely use my patch, no attribution needed. However, if source
> info is desired, you can supply "Ellie <el@horse64.org>" for that.

Can you please provide a patch formatted by git-format-patch(1)?  It
should have a commit message, author, and all that.

> 
> Regards,
> 
> Ellie
> 
> PS: The error that "make" gave me was this one, in case anybody finds this
> useful: "TROFF .tmp/man/man2/s390_sthyi.2.cat.set
> troff:.tmp/man/man2/s390_sthyi.2:124: warning [p 2, 1.8i]: cannot adjust
> line"
> 
> On 12/12/23 9:47 AM, ellie wrote:
> > Dear Alejandro Colomar,
> > 
> > I hope I'm emailing this to the correct place, I found this contact
> > information on https://man7.org/mtk/contact.html regarding man page
> > feedback:
> > 
> > I'm suggesting that the "man 5 proc" page is expanded with a section
> > clarifying /proc/[pid]/self race conditions, I described details and
> > even made a text suggestion here:
> > 
> > https://bugzilla.suse.com/show_bug.cgi?id=1216352
> > 
> > (The text suggestion might be wrong, however, since I don't actually
> > know what the exact technical state of this is.)
> > 
> > Regards,
> > 
> > ellie



-- 
<https://www.alejandro-colomar.es/>

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

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

[Alejandro Colomar]

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

On Sat, Jun 08, 2024 at 12:23:11AM GMT, ellie wrote:
> Dear Alejandro Colomar,
> 
> I finally wrote a patch against the latest man pages, see file attached.
> Hopefully this is in a format that allows you to integrate it well.
> 
> Some quick notes:
> 
> 1. The "make" process aborted with an error, I couldn't use it to verify.
> However, pandoc seems to think my formatting is correct.
> 
> 2. I still don't know if open("/proc/self/exe") has any internal race
> conditions on rename, like any more regular symlink. My text simply assumes
> it doesn't. Maybe some expert might want to double-check?
> 
> 3. You can freely use my patch, no attribution needed. However, if source
> info is desired, you can supply "Ellie <el@horse64.org>" for that.

Also, please send the patch inline.  Otherwise, I have a hard time quote
replying to it.

> 
> Regards,
> 
> Ellie
> 
> PS: The error that "make" gave me was this one, in case anybody finds this
> useful: "TROFF .tmp/man/man2/s390_sthyi.2.cat.set
> troff:.tmp/man/man2/s390_sthyi.2:124: warning [p 2, 1.8i]: cannot adjust
> line"
> 
> On 12/12/23 9:47 AM, ellie wrote:
> > Dear Alejandro Colomar,
> > 
> > I hope I'm emailing this to the correct place, I found this contact
> > information on https://man7.org/mtk/contact.html regarding man page
> > feedback:
> > 
> > I'm suggesting that the "man 5 proc" page is expanded with a section
> > clarifying /proc/[pid]/self race conditions, I described details and
> > even made a text suggestion here:
> > 
> > https://bugzilla.suse.com/show_bug.cgi?id=1216352
> > 
> > (The text suggestion might be wrong, however, since I don't actually
> > know what the exact technical state of this is.)
> > 
> > Regards,
> > 
> > ellie



-- 
<https://www.alejandro-colomar.es/>

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