2019-11-21 07:30:16

by Chen Hu

[permalink] [raw]
Subject: [PATCH] proc: align mnt_id in /proc/pid/fdinfo and /proc/pid/mountinfo

For Android application process, we found that the mnt_id read from
/proc/pid/fdinfo doesn't exist in /proc/pid/mountinfo. Thus CRIU fails
to dump such process and it complains

"(00.019206) Error (criu/files-reg.c:1299): Can't lookup mount=42 for
fd=-3 path=/data/dalvik-cache/x86_64/system@[email protected]"

This is due to how Android application is launched. In Android, there is
a special process called Zygote which handles the forking of each new
application process:
0. Zygote opens and maps some files, for example
"/data/dalvik-cache/x86_64/system@[email protected]" in its current
mount namespace, say "old mnt ns".
1. Zygote waits for the request to fork a new application.
2. Zygote gets a request, it forks and run the new process in a new
mount namespace, say "new mnt ns".

The file opened in step 0 ties to the mount point in "old mnt ns". The
mnt_id of that mount is listed in /proc/pid/fdinfo. However,
/proc/pid/mountinfo points to current ns, i.e., "new mnt ns".

Althgouh this issue is exposed in Android, we believe it's generic.
Prcoess may open files and enter new mnt ns.

To address it, this patch searches the mirror mount in current ns with
MAJOR and MINOR and shows the mirror's mnt_id.

Signed-off-by: Chen, Hu <[email protected]>

diff --git a/fs/mount.h b/fs/mount.h
index 711a4093e475..6bbfc2b3b8ba 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -153,3 +153,5 @@ static inline bool is_anon_ns(struct mnt_namespace *ns)
{
return ns->seq == 0;
}
+
+extern struct mount *lookup_mirror_mnt(const struct mount *mnt);
diff --git a/fs/namespace.c b/fs/namespace.c
index 2adfe7b166a3..4ea9b4464cd0 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -683,6 +683,36 @@ bool __is_local_mountpoint(struct dentry *dentry)
return is_covered;
}

+/*
+ * lookup_mirror_mnt - Return @mnt's mirror mount in the current/local mount
+ * namespace. If mirror isn't found, just return NULL.
+ */
+struct mount *lookup_mirror_mnt(const struct mount *mnt)
+{
+ struct mnt_namespace *ns = current->nsproxy->mnt_ns;
+ struct mount *mnt_local;
+ bool is_matched = false;
+
+ /* mnt belongs to current namesapce */
+ if (mnt->mnt_ns == ns)
+ return mnt;
+
+ down_read(&namespace_sem);
+ list_for_each_entry(mnt_local, &ns->list, mnt_list) {
+ struct super_block *sb = mnt->mnt.mnt_sb;
+ struct super_block *sb_local = mnt_local->mnt.mnt_sb;
+
+ if (MAJOR(sb->s_dev) == MAJOR(sb_local->s_dev) &&
+ MINOR(sb->s_dev) == MINOR(sb_local->s_dev)) {
+ is_matched = true;
+ break;
+ }
+ }
+ up_read(&namespace_sem);
+
+ return is_matched ? mnt_local : NULL;
+}
+
static struct mountpoint *lookup_mountpoint(struct dentry *dentry)
{
struct hlist_head *chain = mp_hash(dentry);
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 81882a13212d..cbf2571b0620 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -23,6 +23,7 @@ static int seq_show(struct seq_file *m, void *v)
int f_flags = 0, ret = -ENOENT;
struct file *file = NULL;
struct task_struct *task;
+ struct mount *mount = NULL;

task = get_proc_task(m->private);
if (!task)
@@ -53,9 +54,16 @@ static int seq_show(struct seq_file *m, void *v)
if (ret)
return ret;

+ /* After unshare -m, real_mount(file->f_path.mnt) is not meaningful in
+ * current mount namesapce. We want to know the mnt_id in current mount
+ * namespace
+ */
+ mount = lookup_mirror_mnt(real_mount(file->f_path.mnt));
+ if (!mount)
+ mount = real_mount(file->f_path.mnt);
+
seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
- (long long)file->f_pos, f_flags,
- real_mount(file->f_path.mnt)->mnt_id);
+ (long long)file->f_pos, f_flags, mount->mnt_id);

show_fd_locks(m, file, files);
if (seq_has_overflowed(m))
--
2.22.0


2019-11-21 10:05:24

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH] proc: align mnt_id in /proc/pid/fdinfo and /proc/pid/mountinfo

On Thu, Nov 21, 2019 at 8:28 AM Chen, Hu <[email protected]> wrote:
>
> For Android application process, we found that the mnt_id read from
> /proc/pid/fdinfo doesn't exist in /proc/pid/mountinfo. Thus CRIU fails
> to dump such process and it complains
>
> "(00.019206) Error (criu/files-reg.c:1299): Can't lookup mount=42 for
> fd=-3 path=/data/dalvik-cache/x86_64/system@[email protected]"
>
> This is due to how Android application is launched. In Android, there is
> a special process called Zygote which handles the forking of each new
> application process:
> 0. Zygote opens and maps some files, for example
> "/data/dalvik-cache/x86_64/system@[email protected]" in its current
> mount namespace, say "old mnt ns".
> 1. Zygote waits for the request to fork a new application.
> 2. Zygote gets a request, it forks and run the new process in a new
> mount namespace, say "new mnt ns".
>
> The file opened in step 0 ties to the mount point in "old mnt ns". The
> mnt_id of that mount is listed in /proc/pid/fdinfo. However,
> /proc/pid/mountinfo points to current ns, i.e., "new mnt ns".
>
> Althgouh this issue is exposed in Android, we believe it's generic.
> Prcoess may open files and enter new mnt ns.
>
> To address it, this patch searches the mirror mount in current ns with
> MAJOR and MINOR and shows the mirror's mnt_id.

This is a hack. I suggest instead to add a new line to fdinfo with
the MAJOR:MINOR number of the device.

Thanks,
Miklos

2019-11-22 03:53:42

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] proc: align mnt_id in /proc/pid/fdinfo and /proc/pid/mountinfo

Hi Hu",

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.4-rc8 next-20191121]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Chen-Hu/proc-align-mnt_id-in-proc-pid-fdinfo-and-proc-pid-mountinfo/20191122-093927
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 81429eb8d9ca40b0c65bb739d29fa856c5d5e958
config: parisc-c3000_defconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=parisc

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All warnings (new ones prefixed by >>):

fs/namespace.c: In function 'lookup_mirror_mnt':
>> fs/namespace.c:698:10: warning: return discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
return mnt;
^~~

vim +/const +698 fs/namespace.c

685
686 /*
687 * lookup_mirror_mnt - Return @mnt's mirror mount in the current/local mount
688 * namespace. If mirror isn't found, just return NULL.
689 */
690 struct mount *lookup_mirror_mnt(const struct mount *mnt)
691 {
692 struct mnt_namespace *ns = current->nsproxy->mnt_ns;
693 struct mount *mnt_local;
694 bool is_matched = false;
695
696 /* mnt belongs to current namesapce */
697 if (mnt->mnt_ns == ns)
> 698 return mnt;
699
700 down_read(&namespace_sem);
701 list_for_each_entry(mnt_local, &ns->list, mnt_list) {
702 struct super_block *sb = mnt->mnt.mnt_sb;
703 struct super_block *sb_local = mnt_local->mnt.mnt_sb;
704
705 if (MAJOR(sb->s_dev) == MAJOR(sb_local->s_dev) &&
706 MINOR(sb->s_dev) == MINOR(sb_local->s_dev)) {
707 is_matched = true;
708 break;
709 }
710 }
711 up_read(&namespace_sem);
712
713 return is_matched ? mnt_local : NULL;
714 }
715

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation


Attachments:
(No filename) (2.54 kB)
.config.gz (15.30 kB)
Download all attachments

2019-11-25 02:51:48

by Chen Hu

[permalink] [raw]
Subject: [PATCH v2] proc: align mnt_id in /proc/pid/fdinfo and /proc/pid/mountinfo

For Android application process, we found that the mnt_id read from
/proc/pid/fdinfo doesn't exist in /proc/pid/mountinfo. Thus CRIU fails
to dump such process and it complains

"(00.019206) Error (criu/files-reg.c:1299): Can't lookup mount=42 for
fd=-3 path=/data/dalvik-cache/x86_64/system@[email protected]"

This is due to how Android application is launched. In Android, there is
a special process called Zygote which handles the forking of each new
application process:
0. Zygote opens and maps some files, for example
"/data/dalvik-cache/x86_64/system@[email protected]" in its current
mount namespace, say "old mnt ns".
1. Zygote waits for the request to fork a new application.
2. Zygote gets a request, it forks and run the new process in a new
mount namespace, say "new mnt ns".

The file opened in step 0 ties to the mount point in "old mnt ns". The
mnt_id of that mount is listed in /proc/pid/fdinfo. However,
/proc/pid/mountinfo points to current ns, i.e., "new mnt ns".

Althgouh this issue is exposed in Android, we believe it's generic.
Prcoess may open files and enter new mnt ns.

To address it, this patch searches the mirror mount in current ns with
MAJOR and MINOR and shows the mirror's mnt_id.

v2: fix warning reported by lkp

Reported-by: kbuild test robot <[email protected]>
Signed-off-by: Chen, Hu <[email protected]>
---
fs/mount.h | 2 ++
fs/namespace.c | 30 ++++++++++++++++++++++++++++++
fs/proc/fd.c | 12 ++++++++++--
3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/fs/mount.h b/fs/mount.h
index 711a4093e475..6bbfc2b3b8ba 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -153,3 +153,5 @@ static inline bool is_anon_ns(struct mnt_namespace *ns)
{
return ns->seq == 0;
}
+
+extern struct mount *lookup_mirror_mnt(const struct mount *mnt);
diff --git a/fs/namespace.c b/fs/namespace.c
index 2adfe7b166a3..131b36517472 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -683,6 +683,36 @@ bool __is_local_mountpoint(struct dentry *dentry)
return is_covered;
}

+/*
+ * lookup_mirror_mnt - Return @mnt's mirror mount in the current/local mount
+ * namespace. If mirror isn't found, just return NULL.
+ */
+struct mount *lookup_mirror_mnt(const struct mount *mnt)
+{
+ struct mnt_namespace *ns = current->nsproxy->mnt_ns;
+ struct mount *mnt_local;
+ bool is_matched = false;
+
+ /* mnt belongs to current namesapce */
+ if (mnt->mnt_ns == ns)
+ return (struct mount *) mnt;
+
+ down_read(&namespace_sem);
+ list_for_each_entry(mnt_local, &ns->list, mnt_list) {
+ struct super_block *sb = mnt->mnt.mnt_sb;
+ struct super_block *sb_local = mnt_local->mnt.mnt_sb;
+
+ if (MAJOR(sb->s_dev) == MAJOR(sb_local->s_dev) &&
+ MINOR(sb->s_dev) == MINOR(sb_local->s_dev)) {
+ is_matched = true;
+ break;
+ }
+ }
+ up_read(&namespace_sem);
+
+ return is_matched ? mnt_local : NULL;
+}
+
static struct mountpoint *lookup_mountpoint(struct dentry *dentry)
{
struct hlist_head *chain = mp_hash(dentry);
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 81882a13212d..cbf2571b0620 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -23,6 +23,7 @@ static int seq_show(struct seq_file *m, void *v)
int f_flags = 0, ret = -ENOENT;
struct file *file = NULL;
struct task_struct *task;
+ struct mount *mount = NULL;

task = get_proc_task(m->private);
if (!task)
@@ -53,9 +54,16 @@ static int seq_show(struct seq_file *m, void *v)
if (ret)
return ret;

+ /* After unshare -m, real_mount(file->f_path.mnt) is not meaningful in
+ * current mount namesapce. We want to know the mnt_id in current mount
+ * namespace
+ */
+ mount = lookup_mirror_mnt(real_mount(file->f_path.mnt));
+ if (!mount)
+ mount = real_mount(file->f_path.mnt);
+
seq_printf(m, "pos:\t%lli\nflags:\t0%o\nmnt_id:\t%i\n",
- (long long)file->f_pos, f_flags,
- real_mount(file->f_path.mnt)->mnt_id);
+ (long long)file->f_pos, f_flags, mount->mnt_id);

show_fd_locks(m, file, files);
if (seq_has_overflowed(m))
--
2.22.0