Hi Askar, Carlos,
On Sun, Feb 16, 2025 at 06:18:28AM +0000, Askar Safin wrote:
> I verified using expirement that modern glibc wrapper getcwd actually never returns "(unreachable)".
> Also I have read modern glibc sources for all 3 functions documented here.
> All they don't return "(unreachable)".
> Also I changed "pathname" to "pathnames".
>
> Now let me describe my expirement. I compiled this C source:
>
> ===
> #include <sys/syscall.h>
> #include <unistd.h>
> #include <stdio.h>
>
> int
> main (void)
> {
> char buf[1000];
> if(syscall(SYS_getcwd, buf, sizeof(buf)) == -1)
> {
> perror ("SYS_getcwd");
> }
> else
> {
> printf ("SYS_getcwd: %s\n", buf);
> }
> if(getcwd(buf, sizeof(buf)) == NULL)
> {
> perror ("getcwd");
> }
> else
> {
> printf ("getcwd: %s\n", buf);
> }
> return 0;
> }
> ===
>
> to a binary /tmp/a and run the following command:
>
> $ sudo unshare --mount bash -c 'set -e; mkdir /tmp/dir; mount -t tmpfs tmpfs /tmp/dir; cd /tmp/dir; umount -l .; /tmp/a'
>
> The output was:
>
> SYS_getcwd: (unreachable)/
> getcwd: No such file or directory
Thanks! I prefer if we show a continuous shell session, which is easier
to follow than code mixed with explanations. So I would add the
following (and indented, to show it's code; that also makes it easier to
keep the #include lines in git(1)):
alx@devuan:~/tmp$ cat getcwd.c;
#include <unistd.h>
#include <stdio.h>
#include <sys/syscall.h>
int
main(void)
{
char buf[1000];
if (syscall(SYS_getcwd, buf, sizeof(buf)) == -1)
perror("SYS_getcwd");
else
printf("SYS_getcwd: %s\n", buf);
if (getcwd(buf, sizeof(buf)) == NULL)
perror("getcwd");
else
printf("getcwd: %s\n", buf);
return 0;
}
alx@devuan:~/tmp$ gcc -Wall -Wextra -o /tmp/getcwd getcwd.c;
alx@devuan:~/tmp$ sudo unshare --mount bash;
root@devuan:/home/alx/tmp# set -e;
root@devuan:/home/alx/tmp# mkdir /tmp/dir;
root@devuan:/home/alx/tmp# mount -t tmpfs tmpfd /tmp/dir/;
root@devuan:/home/alx/tmp# cd /tmp/dir/;
root@devuan:/tmp/dir# umount -l .;
root@devuan:/tmp/dir# /tmp/getcwd;
SYS_getcwd: (unreachable)/
getcwd: No such file or directory
root@devuan:/tmp/dir# exit;
exit
I have also adapted the example program to the C coding style of the
project:
<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING.d/style/c>
>
> Signed-off-by: Askar Safin <safinaskar@xxxxxxxxxxxx>
Please add a line for Carlos:
Cc: Carlos O'Donell <carlos@xxxxxxxxxx>
> ---
>
> Changes since v1:
> - I added that old glibc versions are buggy, too
> - Added sources for my expirement to commit message (and did small tweaks to commit message)
>
> man/man3/getcwd.3 | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/man/man3/getcwd.3 b/man/man3/getcwd.3
> index 685585a60..97e3c766f 100644
> --- a/man/man3/getcwd.3
> +++ b/man/man3/getcwd.3
> @@ -245,8 +245,11 @@ process (e.g., because the process set a new filesystem root using
> without changing its current directory into the new root).
> Such behavior can also be caused by an unprivileged user by changing
> the current directory into another mount namespace.
> -When dealing with pathname from untrusted sources, callers of the
> -functions described in this page
> +When dealing with pathnames from untrusted sources, callers of the
As Carlos said, I prefer if this patch doesn't include the change
s/pathname/&s/. That would reduce the diff.
Thanks for the help with this, Carlos!
> +functions described in this page (until glibc 2.27)
> +or raw
I think it would be more appropriate to say 's/or raw/or the raw/'.
> +.BR getcwd ()
> +system call
Other than those minor comments, I think this LGTM. Please address
those, and I'll take the patch.
Carlos do you have any further comments?
Have a lovely day!
Alex
> should consider checking whether the returned pathname starts
> with '/' or '(' to avoid misinterpreting an unreachable path
> as a relative pathname.
> --
> 2.39.5
>
>
--
<https://www.alejandro-colomar.es/>
Attachment:
signature.asc
Description: PGP signature
