lkml.org 
[lkml]   [2013]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] proc: Fix namespace mountpoint path in /proc/mounts
"Serge E. Hallyn" <serge@hallyn.com> writes:

> Quoting Aditya Kali (adityakali@google.com):
>> Commit bf056bfa80596a5d14b26b17276a56a0dcb080e5:
>> "proc: Fix the namespace inode permission checks." converted
>> the namespace files into symlinks. The same commit changed
>> the way namespace bind mounts appear in /proc/mounts:
>> $ mount --bind /proc/self/ns/ipc /mnt/ipc
>> Originally:
>> $ cat /proc/mounts | grep ipc
>> proc /mnt/ipc proc rw,nosuid,nodev,noexec 0 0
>>
>> After commit bf056bfa80596a5d14b26b17276a56a0dcb080e5:
>> $ cat /proc/mounts | grep ipc
>> proc ipc:[4026531839] proc rw,nosuid,nodev,noexec 0 0
>>
>> This breaks userspace which expects the 2nd field in
>> /proc/mounts to be a valid path. This patch restores the
>> original format of namespace bind-mount entries in
>> /proc/mounts.
>
> Oh, at first I was thinking the mount source was showing up like that,
> not the target. That is particularly ugly, I agree.
>
> I'm not sure what the purpose was of the ns_dname(). dcache.c says it's
> for filesystems wanting to do 'special "root names"'. But this file
> gets mounted to real paths, it's not actually rootless (like a pipefs
> inode or anon_inode). So I think your patch is correct, but I'm waiting
> to hear from Eric, as I'm not sure if you're masking some other effect
> which Eric actually wanted, and maybe this should be fixed another
> way...

My apologies for taking a long time to get back to this one. I have
been scratching my head on this one.

There is most definitely a bug here, and worth fixing.

But I believe the bug is actually in buried in /proc/mounts. ns_dname
should be irrelevant as we are mounted.

The problem comes down to d_path.

I am not certain which is the best fix at the moment. It should either
be a case of fixing d_path to see if the dentry is mounted, or making
certain that the dentries have the name ns_dname is giving them when
we allocate the dentries.

I was focusing on the what a ns file descriptor should look like when it
is opened but not mounted, when I wrote ns_dname, and that appearance
really should continue if possible.

I expect the easist way to fix this is to simply modify proc_ns_get_dentry to
compute the dentry name that ns_dname uses today, and pass that name to
d_alloc_psuedo.

At which point we can delete ns_dname without problems.

Eric


>> Signed-off-by: Aditya Kali <adityakali@google.com>
>> ---
>> fs/proc/namespaces.c | 10 ----------
>> 1 file changed, 10 deletions(-)
>>
>> diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
>> index 49a7fff..d19989d 100644
>> --- a/fs/proc/namespaces.c
>> +++ b/fs/proc/namespaces.c
>> @@ -48,19 +48,9 @@ static int ns_delete_dentry(const struct dentry *dentry)
>> return 1;
>> }
>>
>> -static char *ns_dname(struct dentry *dentry, char *buffer, int buflen)
>> -{
>> - struct inode *inode = dentry->d_inode;
>> - const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns.ns_ops;
>> -
>> - return dynamic_dname(dentry, buffer, buflen, "%s:[%lu]",
>> - ns_ops->name, inode->i_ino);
>> -}
>> -
>> const struct dentry_operations ns_dentry_operations =
>> {
>> .d_delete = ns_delete_dentry,
>> - .d_dname = ns_dname,
>> };
>>
>> static struct dentry *proc_ns_get_dentry(struct super_block *sb,
>> --
>> 1.8.4.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/


\
 
 \ /
  Last update: 2013-11-09 01:41    [W:0.068 / U:0.560 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site