"Serge E. Hallyn" <serge@xxxxxxxxxx> writes:
> Quoting Aditya Kali (adityakali@xxxxxxxxxx):
>> 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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
