Hi Askar,
On Thu, Feb 20, 2025 at 09:19:25AM +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)".
We have a (soft) 72-col limit on the commit message. I should document
this, as I think it's not documented anywhere. I also took the time to
fix some minor wording issues.
(That limit doesn't apply to the subject, which can be of arbitrary
lenght, as long as it's reasonable.)
> Now let me describe my expirement:
>
> d-user@comp:/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;
> }
> d-user@comp:/tmp$ gcc -Wall -Wextra -o getcwd getcwd.c
> d-user@comp:/tmp$ sudo unshare --mount bash
> d-root@comp:/tmp# mkdir /tmp/dir
> d-root@comp:/tmp# mount -t tmpfs tmpfs /tmp/dir
> d-root@comp:/tmp# cd /tmp/dir
> d-root@comp:/tmp/dir# umount -l .
> d-root@comp:/tmp/dir# /tmp/getcwd
> SYS_getcwd: (unreachable)/
> getcwd: No such file or directory
> d-root@comp:/tmp/dir# exit
> exit
>
> Reviewed-by: Carlos O'Donell <carlos@xxxxxxxxxx>
> Link: <https://sourceware.org/bugzilla/show_bug.cgi?id=18203>
> Link: <https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=52a713fdd0a30e1bd79818e2e3c4ab44ddca1a94>
I moved the R-B tag to after the links, since those links are also
reviewed.
> Signed-off-by: Askar Safin <safinaskar@xxxxxxxxxxxx>
> ---
> man/man3/getcwd.3 | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/man/man3/getcwd.3 b/man/man3/getcwd.3
> index 685585a60..919ffb08f 100644
> --- a/man/man3/getcwd.3
> +++ b/man/man3/getcwd.3
> @@ -246,7 +246,10 @@ 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
> +functions described in this page (until glibc 2.27)
I changed this from 'until' to 'before'. The word until doesn't make it
clear if the version 2.27 has the old or the new behavior. The word
before makes it clear that the old behavior was not present in 2.27.
As I mentioned as a reply to the cover letter, I've applied this patch,
and amended it editorially with those minor edits. Thanks!
Cheers,
Alex
> +or the raw
> +.BR getcwd ()
> +system call
> 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
