On Tue, Feb 18, 2025 at 06:37:09PM -0500, Carlos O'Donell wrote:
> On 2/17/25 11:51 AM, 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".
>
> Commit message is incorrect, but the rest of the change looks good.
>
> Please drop the "Also I changed..." line since you made this change in patch 2/2.
>
> You can keep my RB if you drop the line.
>
> Reviewed-by: Carlos O'Donell <carlos@xxxxxxxxxx>
Thanks!
>
> >
> > 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
> >
> > Cc: 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>
> > 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)
> > +or the raw
>
> OK. Includes Alejandro's suggestion.
>
> > +.BR getcwd ()
> > +system call
> > should consider checking whether the returned pathname starts
> > with '/' or '(' to avoid misinterpreting an unreachable path
> > as a relative pathname.
>
>
> --
> Cheers,
> Carlos.
>
--
<https://www.alejandro-colomar.es/>
Attachment:
signature.asc
Description: PGP signature
