Re: [PATCH 2/2] linux-user: Fix openat() emulation to not modify atime

[Shu-Chun Weng]

On Mon, Dec 4, 2023 at 8:58 AM Daniel P. Berrangé 
wrote:

> On Thu, Nov 30, 2023 at 07:21:40PM -0800, Shu-Chun Weng wrote:
> > Commit b8002058 strengthened openat()'s /proc detection by calling
> > realpath(3) on the given path, which allows various paths and symlinks
> > that points to the /proc file system to be intercepted correctly.
> >
> > Using realpath(3), though, has a side effect that it reads the symlinks
> > along the way, and thus changes their atime. The results in the
> > following code snippet already get ~now instead of the real atime:
> >
> >   int fd = open("/path/to/a/symlink", O_PATH | O_NOFOLLOW);
> >   struct stat st;
> >   fstat(fd, st);
> >   return st.st_atime;
> >
> > This change opens a path that doesn't appear to be part of /proc
> > directly and checks the destination of /proc/self/fd/n to determine if
> > it actually refers to a file in /proc.
> >
> > Neither this nor the existing code works with symlinks or indirect paths
> > (e.g.  /tmp/../proc/self/exe) that points to /proc/self/exe because it
> > is itself a symlink, and both realpath(3) and /proc/self/fd/n will
> > resolve into the location of QEMU.
>
> I wonder if we can detect that by opening with O_NOFOLLOW, then
> calling fstatfs() on the FD, and checking f_type == PROCFS_SUPER_MAGIC
>

This works with indirect or relative paths, yes, but still not symlinks. I
decided not to complicate the logic further.


>
>
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index e384e14248..25e2cda10a 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -8308,8 +8308,6 @@ static int open_net_route(CPUArchState *cpu_env,
> int fd)
> >  int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *fname,
> >  int flags, mode_t mode, bool safe)
> >  {
> > -g_autofree char *proc_name = NULL;
> > -const char *pathname;
> >  struct fake_open {
> >  const char *filename;
> >  int (*fill)(CPUArchState *cpu_env, int fd);
> > @@ -8333,13 +8331,39 @@ int do_guest_openat(CPUArchState *cpu_env, int
> dirfd, const char *fname,
> >  #endif
> >  { NULL, NULL, NULL }
> >  };
> > +char pathname[PATH_MAX];
> >
> > -/* if this is a file from /proc/ filesystem, expand full name */
> > -proc_name = realpath(fname, NULL);
> > -if (proc_name && strncmp(proc_name, "/proc/", 6) == 0) {
> > -pathname = proc_name;
> > +if (strncmp(fname, "/proc/", 6) == 0) {
> > +pstrcpy(pathname, sizeof(pathname), fname);
> >  } else {
> > -pathname = fname;
> > +char procpath[PATH_MAX];
> > +int fd, n;
> > +
> > +if (safe) {
> > +fd = safe_openat(dirfd, path(fname), flags, mode);
> > +} else {
> > +fd = openat(dirfd, path(fname), flags, mode);
> > +}
> > +if (fd < 0) {
> > +return fd;
> > +}
> > +
> > +/*
> > + * Try to get the real path of the file we just opened. We
> avoid calling
> > + * `realpath(3)` because it calls `readlink(2)` on symlinks
> which
> > + * changes their atime. Note that since `/proc/self/exe` is a
> symlink,
> > + * `pathname` will never resolves to it (neither will
> `realpath(3)`).
> > + * That's why we check `fname` against the "/proc/" prefix
> first.
> > + */
> > +snprintf(procpath, sizeof(procpath), "/proc/self/fd/%d", fd);
>
> g_strdup_printf() + g_autofree to avoid this PATH_MAX buffer
>
> > +n = readlink(procpath, pathname, sizeof(pathname));
> > +pathname[n < sizeof(pathname) ? n : sizeof(pathname)] = '\0';
>
> If you call lstat() then sb_size will tell you how big the buffer
> needs to be for a subsequent readlink(), whcih can be allocated
> on the heap and released with g_autofree, avoiding the othuer PATH_MAX
> buffer
>

Thanks for the suggestions, sent out v2 of the patch series eliminating
both static buffers.

Shu-Chun


> > +
> > +/* if this is not a file from /proc/ filesystem, the fd is good
> as-is */
> > +if (strncmp(pathname, "/proc/", 6) != 0) {
> > +return fd;
> > +}
> > +close(fd);
> >  }
> >
> >  if (is_proc_myself(pathname, "exe")) {
> > @@ -8390,9 +8414,9 @@ int do_guest_openat(CPUArchState *cpu_env, int
> dirfd, const char *fname,
> >  }
> >
> >  if (safe) {
> > -return safe_openat(dirfd, path(pathname), flags, mode);
> > +return safe_openat(dirfd, pathname, flags, mode);
> >  } else {
> > -return openat(dirfd, path(pathname), flags, mode);
> > +return openat(dirfd, pathname, flags, mode);
> >  }
> >  }
> >
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com  -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
>
>


smime.p7s
Description: S/MIME 

Re: [PATCH 2/2] linux-user: Fix openat() emulation to not modify atime

[Daniel P . Berrangé]

On Thu, Nov 30, 2023 at 07:21:40PM -0800, Shu-Chun Weng wrote:
> Commit b8002058 strengthened openat()'s /proc detection by calling
> realpath(3) on the given path, which allows various paths and symlinks
> that points to the /proc file system to be intercepted correctly.
> 
> Using realpath(3), though, has a side effect that it reads the symlinks
> along the way, and thus changes their atime. The results in the
> following code snippet already get ~now instead of the real atime:
> 
>   int fd = open("/path/to/a/symlink", O_PATH | O_NOFOLLOW);
>   struct stat st;
>   fstat(fd, st);
>   return st.st_atime;
> 
> This change opens a path that doesn't appear to be part of /proc
> directly and checks the destination of /proc/self/fd/n to determine if
> it actually refers to a file in /proc.
> 
> Neither this nor the existing code works with symlinks or indirect paths
> (e.g.  /tmp/../proc/self/exe) that points to /proc/self/exe because it
> is itself a symlink, and both realpath(3) and /proc/self/fd/n will
> resolve into the location of QEMU.

I wonder if we can detect that by opening with O_NOFOLLOW, then
calling fstatfs() on the FD, and checking f_type == PROCFS_SUPER_MAGIC


> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e384e14248..25e2cda10a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8308,8 +8308,6 @@ static int open_net_route(CPUArchState *cpu_env, int fd)
>  int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *fname,
>  int flags, mode_t mode, bool safe)
>  {
> -g_autofree char *proc_name = NULL;
> -const char *pathname;
>  struct fake_open {
>  const char *filename;
>  int (*fill)(CPUArchState *cpu_env, int fd);
> @@ -8333,13 +8331,39 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, 
> const char *fname,
>  #endif
>  { NULL, NULL, NULL }
>  };
> +char pathname[PATH_MAX];
>  
> -/* if this is a file from /proc/ filesystem, expand full name */
> -proc_name = realpath(fname, NULL);
> -if (proc_name && strncmp(proc_name, "/proc/", 6) == 0) {
> -pathname = proc_name;
> +if (strncmp(fname, "/proc/", 6) == 0) {
> +pstrcpy(pathname, sizeof(pathname), fname);
>  } else {
> -pathname = fname;
> +char procpath[PATH_MAX];
> +int fd, n;
> +
> +if (safe) {
> +fd = safe_openat(dirfd, path(fname), flags, mode);
> +} else {
> +fd = openat(dirfd, path(fname), flags, mode);
> +}
> +if (fd < 0) {
> +return fd;
> +}
> +
> +/*
> + * Try to get the real path of the file we just opened. We avoid 
> calling
> + * `realpath(3)` because it calls `readlink(2)` on symlinks which
> + * changes their atime. Note that since `/proc/self/exe` is a 
> symlink,
> + * `pathname` will never resolves to it (neither will `realpath(3)`).
> + * That's why we check `fname` against the "/proc/" prefix first.
> + */
> +snprintf(procpath, sizeof(procpath), "/proc/self/fd/%d", fd);

g_strdup_printf() + g_autofree to avoid this PATH_MAX buffer

> +n = readlink(procpath, pathname, sizeof(pathname));
> +pathname[n < sizeof(pathname) ? n : sizeof(pathname)] = '\0';

If you call lstat() then sb_size will tell you how big the buffer
needs to be for a subsequent readlink(), whcih can be allocated
on the heap and released with g_autofree, avoiding the othuer PATH_MAX
buffer

> +
> +/* if this is not a file from /proc/ filesystem, the fd is good 
> as-is */
> +if (strncmp(pathname, "/proc/", 6) != 0) {
> +return fd;
> +}
> +close(fd);
>  }
>  
>  if (is_proc_myself(pathname, "exe")) {
> @@ -8390,9 +8414,9 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, 
> const char *fname,
>  }
>  
>  if (safe) {
> -return safe_openat(dirfd, path(pathname), flags, mode);
> +return safe_openat(dirfd, pathname, flags, mode);
>  } else {
> -return openat(dirfd, path(pathname), flags, mode);
> +return openat(dirfd, pathname, flags, mode);
>  }
>  }
>  
> 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH 2/2] linux-user: Fix openat() emulation to not modify atime

[Stefan Hajnoczi]

On Mon, Dec 04, 2023 at 02:39:24PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Laurent, Helge, Richard,
> 
> On 1/12/23 19:51, Shu-Chun Weng wrote:
> > On Fri, Dec 1, 2023 at 4:42 AM Philippe Mathieu-Daudé  > > wrote:
> > 
> > Hi Shu-Chun,
> > 
> > On 1/12/23 04:21, Shu-Chun Weng wrote:
> >  > Commit b8002058 strengthened openat()'s /proc detection by calling
> >  > realpath(3) on the given path, which allows various paths and
> > symlinks
> >  > that points to the /proc file system to be intercepted correctly.
> >  >
> >  > Using realpath(3), though, has a side effect that it reads the
> > symlinks
> >  > along the way, and thus changes their atime. The results in the
> >  > following code snippet already get ~now instead of the real atime:
> >  >
> >  >    int fd = open("/path/to/a/symlink", O_PATH | O_NOFOLLOW);
> >  >    struct stat st;
> >  >    fstat(fd, st);
> >  >    return st.st_atime;
> >  >
> >  > This change opens a path that doesn't appear to be part of /proc
> >  > directly and checks the destination of /proc/self/fd/n to
> > determine if
> >  > it actually refers to a file in /proc.
> >  >
> >  > Neither this nor the existing code works with symlinks or
> > indirect paths
> >  > (e.g.  /tmp/../proc/self/exe) that points to /proc/self/exe
> > because it
> >  > is itself a symlink, and both realpath(3) and /proc/self/fd/n will
> >  > resolve into the location of QEMU.
> > 
> > Does this fix any of the following issues?
> > https://gitlab.com/qemu-project/qemu/-/issues/829
> > 
> > 
> > 
> > Not this one -- this is purely in the logic of util/path.c, which we do
> > see and carry an internal patch. It's quite a behavior change so we
> > never upstreamed it.
> > 
> > https://gitlab.com/qemu-project/qemu/-/issues/927
> > 
> > 
> > 
> > No, either. This patch only touches the path handling, not how files are
> > opened.
> > 
> > https://gitlab.com/qemu-project/qemu/-/issues/2004
> > 
> > 
> > 
> > Yes! Though I don't have a toolchain for HPPA or any of the
> > architectures intercepting /proc/cpuinfo handy, I hacked the condition
> > and confirmed that on 7.1 and 8.2, test.c as attached in the bug prints
> > out the host cpuinfo while with this patch, it prints out the content
> > generated by `open_cpuinfo()`.
> > 
> > 
> > 
> >  > Signed-off-by: Shu-Chun Weng  > >
> > 
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2004
> > 
> 
> Do we need to merge this for 8.2?

Please assign release blocker issues to the 8.2 milestone so that are
tracked:
https://gitlab.com/qemu-project/qemu/-/milestones/10

Thanks,
Stefan

> 
> > 
> >  > ---
> >  >   linux-user/syscall.c | 42
> > +-
> >  >   1 file changed, 33 insertions(+), 9 deletions(-)
> > 
> > 
> > On Fri, Dec 1, 2023 at 9:09 AM Helge Deller  > > wrote:
> > 
> > On 12/1/23 04:21, Shu-Chun Weng wrote:
> >  > Commit b8002058 strengthened openat()'s /proc detection by calling
> >  > realpath(3) on the given path, which allows various paths and
> > symlinks
> >  > that points to the /proc file system to be intercepted correctly.
> >  >
> >  > Using realpath(3), though, has a side effect that it reads the
> > symlinks
> >  > along the way, and thus changes their atime.
> > 
> > Ah, ok. I didn't thought of that side effect when I came up with the
> > patch.
> > Does the updated atimes trigger some real case issue ?
> > 
> > 
> > We have an internal library shimming the underlying filesystem that uses
> > the `open(O_PATH|O_NOFOLLOW)`+`fstat()` pattern for all file stats.
> > Checking symlink atime is in one of the unittests, though I don't know
> > if production ever uses it.
> > 
> > 
> > Helge
> > 
> 


signature.asc
Description: PGP signature

Re: [PATCH 2/2] linux-user: Fix openat() emulation to not modify atime

[Philippe Mathieu-Daudé]

Hi Laurent, Helge, Richard,

On 1/12/23 19:51, Shu-Chun Weng wrote:
On Fri, Dec 1, 2023 at 4:42 AM Philippe Mathieu-Daudé > wrote:


Hi Shu-Chun,

On 1/12/23 04:21, Shu-Chun Weng wrote:
 > Commit b8002058 strengthened openat()'s /proc detection by calling
 > realpath(3) on the given path, which allows various paths and
symlinks
 > that points to the /proc file system to be intercepted correctly.
 >
 > Using realpath(3), though, has a side effect that it reads the
symlinks
 > along the way, and thus changes their atime. The results in the
 > following code snippet already get ~now instead of the real atime:
 >
 >    int fd = open("/path/to/a/symlink", O_PATH | O_NOFOLLOW);
 >    struct stat st;
 >    fstat(fd, st);
 >    return st.st_atime;
 >
 > This change opens a path that doesn't appear to be part of /proc
 > directly and checks the destination of /proc/self/fd/n to
determine if
 > it actually refers to a file in /proc.
 >
 > Neither this nor the existing code works with symlinks or
indirect paths
 > (e.g.  /tmp/../proc/self/exe) that points to /proc/self/exe
because it
 > is itself a symlink, and both realpath(3) and /proc/self/fd/n will
 > resolve into the location of QEMU.

Does this fix any of the following issues?
https://gitlab.com/qemu-project/qemu/-/issues/829



Not this one -- this is purely in the logic of util/path.c, which we do 
see and carry an internal patch. It's quite a behavior change so we 
never upstreamed it.


https://gitlab.com/qemu-project/qemu/-/issues/927



No, either. This patch only touches the path handling, not how files are 
opened.


https://gitlab.com/qemu-project/qemu/-/issues/2004



Yes! Though I don't have a toolchain for HPPA or any of the 
architectures intercepting /proc/cpuinfo handy, I hacked the condition 
and confirmed that on 7.1 and 8.2, test.c as attached in the bug prints 
out the host cpuinfo while with this patch, it prints out the content 
generated by `open_cpuinfo()`.




 > Signed-off-by: Shu-Chun Weng mailto:s...@google.com>>


Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2004 



Do we need to merge this for 8.2?



 > ---
 >   linux-user/syscall.c | 42
+-
 >   1 file changed, 33 insertions(+), 9 deletions(-)


On Fri, Dec 1, 2023 at 9:09 AM Helge Deller > wrote:


On 12/1/23 04:21, Shu-Chun Weng wrote:
 > Commit b8002058 strengthened openat()'s /proc detection by calling
 > realpath(3) on the given path, which allows various paths and
symlinks
 > that points to the /proc file system to be intercepted correctly.
 >
 > Using realpath(3), though, has a side effect that it reads the
symlinks
 > along the way, and thus changes their atime.

Ah, ok. I didn't thought of that side effect when I came up with the
patch.
Does the updated atimes trigger some real case issue ?


We have an internal library shimming the underlying filesystem that uses 
the `open(O_PATH|O_NOFOLLOW)`+`fstat()` pattern for all file stats. 
Checking symlink atime is in one of the unittests, though I don't know 
if production ever uses it.



Helge

Re: [PATCH 2/2] linux-user: Fix openat() emulation to not modify atime

[Shu-Chun Weng]

On Fri, Dec 1, 2023 at 4:42 AM Philippe Mathieu-Daudé 
wrote:

> Hi Shu-Chun,
>
> On 1/12/23 04:21, Shu-Chun Weng wrote:
> > Commit b8002058 strengthened openat()'s /proc detection by calling
> > realpath(3) on the given path, which allows various paths and symlinks
> > that points to the /proc file system to be intercepted correctly.
> >
> > Using realpath(3), though, has a side effect that it reads the symlinks
> > along the way, and thus changes their atime. The results in the
> > following code snippet already get ~now instead of the real atime:
> >
> >int fd = open("/path/to/a/symlink", O_PATH | O_NOFOLLOW);
> >struct stat st;
> >fstat(fd, st);
> >return st.st_atime;
> >
> > This change opens a path that doesn't appear to be part of /proc
> > directly and checks the destination of /proc/self/fd/n to determine if
> > it actually refers to a file in /proc.
> >
> > Neither this nor the existing code works with symlinks or indirect paths
> > (e.g.  /tmp/../proc/self/exe) that points to /proc/self/exe because it
> > is itself a symlink, and both realpath(3) and /proc/self/fd/n will
> > resolve into the location of QEMU.
>
> Does this fix any of the following issues?
> https://gitlab.com/qemu-project/qemu/-/issues/829


Not this one -- this is purely in the logic of util/path.c, which we do see
and carry an internal patch. It's quite a behavior change so we never
upstreamed it.


> https://gitlab.com/qemu-project/qemu/-/issues/927


No, either. This patch only touches the path handling, not how files are
opened.

https://gitlab.com/qemu-project/qemu/-/issues/2004


Yes! Though I don't have a toolchain for HPPA or any of the architectures
intercepting /proc/cpuinfo handy, I hacked the condition and confirmed that
on 7.1 and 8.2, test.c as attached in the bug prints out the host cpuinfo
while with this patch, it prints out the content generated by
`open_cpuinfo()`.


>
>
> > Signed-off-by: Shu-Chun Weng 
>

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2004

> ---
> >   linux-user/syscall.c | 42 +-
> >   1 file changed, 33 insertions(+), 9 deletions(-)
>
>
On Fri, Dec 1, 2023 at 9:09 AM Helge Deller  wrote:

> On 12/1/23 04:21, Shu-Chun Weng wrote:
> > Commit b8002058 strengthened openat()'s /proc detection by calling
> > realpath(3) on the given path, which allows various paths and symlinks
> > that points to the /proc file system to be intercepted correctly.
> >
> > Using realpath(3), though, has a side effect that it reads the symlinks
> > along the way, and thus changes their atime.
>
> Ah, ok. I didn't thought of that side effect when I came up with the patch.
> Does the updated atimes trigger some real case issue ?
>

We have an internal library shimming the underlying filesystem that uses
the `open(O_PATH|O_NOFOLLOW)`+`fstat()` pattern for all file stats.
Checking symlink atime is in one of the unittests, though I don't know if
production ever uses it.


>
> Helge
>


smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH 2/2] linux-user: Fix openat() emulation to not modify atime

[Helge Deller]

On 12/1/23 04:21, Shu-Chun Weng wrote:

Commit b8002058 strengthened openat()'s /proc detection by calling
realpath(3) on the given path, which allows various paths and symlinks
that points to the /proc file system to be intercepted correctly.

Using realpath(3), though, has a side effect that it reads the symlinks
along the way, and thus changes their atime.


Ah, ok. I didn't thought of that side effect when I came up with the patch.
Does the updated atimes trigger some real case issue ?

Helge


The results in the
following code snippet already get ~now instead of the real atime:

   int fd = open("/path/to/a/symlink", O_PATH | O_NOFOLLOW);
   struct stat st;
   fstat(fd, st);
   return st.st_atime;

This change opens a path that doesn't appear to be part of /proc
directly and checks the destination of /proc/self/fd/n to determine if
it actually refers to a file in /proc.

Neither this nor the existing code works with symlinks or indirect paths
(e.g.  /tmp/../proc/self/exe) that points to /proc/self/exe because it
is itself a symlink, and both realpath(3) and /proc/self/fd/n will
resolve into the location of QEMU.

Signed-off-by: Shu-Chun Weng 
---
  linux-user/syscall.c | 42 +-
  1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e384e14248..25e2cda10a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8308,8 +8308,6 @@ static int open_net_route(CPUArchState *cpu_env, int fd)
  int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *fname,
  int flags, mode_t mode, bool safe)
  {
-g_autofree char *proc_name = NULL;
-const char *pathname;
  struct fake_open {
  const char *filename;
  int (*fill)(CPUArchState *cpu_env, int fd);
@@ -8333,13 +8331,39 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, 
const char *fname,
  #endif
  { NULL, NULL, NULL }
  };
+char pathname[PATH_MAX];

-/* if this is a file from /proc/ filesystem, expand full name */
-proc_name = realpath(fname, NULL);
-if (proc_name && strncmp(proc_name, "/proc/", 6) == 0) {
-pathname = proc_name;
+if (strncmp(fname, "/proc/", 6) == 0) {
+pstrcpy(pathname, sizeof(pathname), fname);
  } else {
-pathname = fname;
+char procpath[PATH_MAX];
+int fd, n;
+
+if (safe) {
+fd = safe_openat(dirfd, path(fname), flags, mode);
+} else {
+fd = openat(dirfd, path(fname), flags, mode);
+}
+if (fd < 0) {
+return fd;
+}
+
+/*
+ * Try to get the real path of the file we just opened. We avoid 
calling
+ * `realpath(3)` because it calls `readlink(2)` on symlinks which
+ * changes their atime. Note that since `/proc/self/exe` is a symlink,
+ * `pathname` will never resolves to it (neither will `realpath(3)`).
+ * That's why we check `fname` against the "/proc/" prefix first.
+ */
+snprintf(procpath, sizeof(procpath), "/proc/self/fd/%d", fd);
+n = readlink(procpath, pathname, sizeof(pathname));
+pathname[n < sizeof(pathname) ? n : sizeof(pathname)] = '\0';
+
+/* if this is not a file from /proc/ filesystem, the fd is good as-is 
*/
+if (strncmp(pathname, "/proc/", 6) != 0) {
+return fd;
+}
+close(fd);
  }

  if (is_proc_myself(pathname, "exe")) {
@@ -8390,9 +8414,9 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, 
const char *fname,
  }

  if (safe) {
-return safe_openat(dirfd, path(pathname), flags, mode);
+return safe_openat(dirfd, pathname, flags, mode);
  } else {
-return openat(dirfd, path(pathname), flags, mode);
+return openat(dirfd, pathname, flags, mode);
  }
  }

Re: [PATCH 2/2] linux-user: Fix openat() emulation to not modify atime

[Philippe Mathieu-Daudé]

Hi Shu-Chun,

On 1/12/23 04:21, Shu-Chun Weng wrote:

Commit b8002058 strengthened openat()'s /proc detection by calling
realpath(3) on the given path, which allows various paths and symlinks
that points to the /proc file system to be intercepted correctly.

Using realpath(3), though, has a side effect that it reads the symlinks
along the way, and thus changes their atime. The results in the
following code snippet already get ~now instead of the real atime:

   int fd = open("/path/to/a/symlink", O_PATH | O_NOFOLLOW);
   struct stat st;
   fstat(fd, st);
   return st.st_atime;

This change opens a path that doesn't appear to be part of /proc
directly and checks the destination of /proc/self/fd/n to determine if
it actually refers to a file in /proc.

Neither this nor the existing code works with symlinks or indirect paths
(e.g.  /tmp/../proc/self/exe) that points to /proc/self/exe because it
is itself a symlink, and both realpath(3) and /proc/self/fd/n will
resolve into the location of QEMU.


Does this fix any of the following issues?
https://gitlab.com/qemu-project/qemu/-/issues/829
https://gitlab.com/qemu-project/qemu/-/issues/927
https://gitlab.com/qemu-project/qemu/-/issues/2004


Signed-off-by: Shu-Chun Weng 
---
  linux-user/syscall.c | 42 +-
  1 file changed, 33 insertions(+), 9 deletions(-)