On Fri, 2025-08-01 at 22:59 +0530, Kotresh Hiremath Ravishankar wrote: > > Hi, > > 1. I will modify the commit message to clearly explain the issue in the next revision. > 2. The maximum possible length for the fsname is not defined in mds side. I didn't find any restriction imposed on the length. So we need to live with it. We have two constants in Linux kernel [1]: #define NAME_MAX 255 /* # chars in a file name */ #define PATH_MAX 4096 /* # chars in a path name including nul */ I don't think that fsname can be bigger than PATH_MAX. > 3. I will fix up doutc in the next revision. > 4. The fs_name is part of the mdsmap in the server side [1]. The kernel client decodes only necessary fields from the mdsmap sent by the server. Until now, the fs_name > was not being decoded, as part of this fix, it's required and being decoded. > Correct me if I am wrong. I can create a Ceph cluster with several MDS servers. In this cluster, I can create multiple file system volumes. And every file system volume will have some name (fs_name). So, if we store fs_name into mdsmap, then which name do we imply here? Do we imply cluster name as fs_name or name of particular file system volume? Thanks, Slava. > > [1] https://elixir.bootlin.com/linux/v6.16/source/include/uapi/linux/limits.h#L12 > [1] https://github.com/ceph/ceph/blob/main/src/mds/MDSMap.h#L596 > > On Tue, Jul 29, 2025 at 11:57 PM Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx> wrote: > > On Tue, 2025-07-29 at 22:32 +0530, khiremat@xxxxxxxxxx wrote: > > > From: Kotresh HR <khiremat@xxxxxxxxxx> > > > > > > The mds auth caps check should also validate the > > > fsname along with the associated caps. Not doing > > > so would result in applying the mds auth caps of > > > one fs on to the other fs in a multifs ceph cluster. > > > The patch fixes the same. > > > > > > Steps to Reproduce (on vstart cluster): > > > 1. Create two file systems in a cluster, say 'a' and 'b' > > > 2. ceph fs authorize a client.usr / r > > > 3. ceph fs authorize b client.usr / rw > > > 4. ceph auth get client.usr >> ./keyring > > > 5. sudo bin/mount.ceph usr@.a=/ /kmnt_a_usr/ > > > 6. touch /kmnt_a_usr/file1 (SHOULD NOT BE ALLOWED) > > > 7. sudo bin/mount.ceph admin@.a=/ /kmnt_a_admin > > > 8. echo "data" > /kmnt_a_admin/admin_file1 > > > 9. rm -f /kmnt_a_usr/admin_file1 (SHOULD NOT BE ALLOWED) > > > > > > > I think we are missing to explain here which problem or > > symptoms will see the user that has this issue. I assume that > > this will be seen as the issue reproduction: > > > > With client_3 which has only 1 filesystem in caps is working as expected > > mkdir /mnt/client_3/test_3 > > mkdir: cannot create directory ‘/mnt/client_3/test_3’: Permission denied > > > > Am I correct here? > > > > > URL: https://tracker.ceph.com/issues/72167 ; > > > Signed-off-by: Kotresh HR <khiremat@xxxxxxxxxx> > > > --- > > > fs/ceph/mds_client.c | 10 ++++++++++ > > > fs/ceph/mdsmap.c | 11 ++++++++++- > > > fs/ceph/mdsmap.h | 1 + > > > 3 files changed, 21 insertions(+), 1 deletion(-) > > > > > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > > > index 9eed6d73a508..ba91f3360ff6 100644 > > > --- a/fs/ceph/mds_client.c > > > +++ b/fs/ceph/mds_client.c > > > @@ -5640,11 +5640,21 @@ static int ceph_mds_auth_match(struct ceph_mds_client *mdsc, > > > u32 caller_uid = from_kuid(&init_user_ns, cred->fsuid); > > > u32 caller_gid = from_kgid(&init_user_ns, cred->fsgid); > > > struct ceph_client *cl = mdsc->fsc->client; > > > + const char *fs_name = mdsc->mdsmap->fs_name; > > > const char *spath = mdsc->fsc->mount_options->server_path; > > > bool gid_matched = false; > > > u32 gid, tlen, len; > > > int i, j; > > > > > > + if (auth->match.fs_name && strcmp(auth->match.fs_name, fs_name)) { > > > > Should we consider to use strncmp() here? > > We should have the limitation of maximum possible name length. > > > > > + doutc(cl, "fsname check failed fs_name=%s match.fs_name=%s\n", > > > + fs_name, auth->match.fs_name); > > > + return 0; > > > + } else { > > > + doutc(cl, "fsname check passed fs_name=%s match.fs_name=%s\n", > > > + fs_name, auth->match.fs_name); > > > > I assume that we could call the doutc with auth->match.fs_name == NULL. So, I am > > expecting to have a crash here. > > > > > + } > > > + > > > doutc(cl, "match.uid %lld\n", auth->match.uid); > > > if (auth->match.uid != MDS_AUTH_UID_ANY) { > > > if (auth->match.uid != caller_uid) > > > diff --git a/fs/ceph/mdsmap.c b/fs/ceph/mdsmap.c > > > index 8109aba66e02..f1431ba0b33e 100644 > > > --- a/fs/ceph/mdsmap.c > > > +++ b/fs/ceph/mdsmap.c > > > @@ -356,7 +356,15 @@ struct ceph_mdsmap *ceph_mdsmap_decode(struct ceph_mds_client *mdsc, void **p, > > > /* enabled */ > > > ceph_decode_8_safe(p, end, m->m_enabled, bad_ext); > > > /* fs_name */ > > > - ceph_decode_skip_string(p, end, bad_ext); > > > + m->fs_name = ceph_extract_encoded_string(p, end, NULL, GFP_NOFS); > > > + if (IS_ERR(m->fs_name)) { > > > + err = PTR_ERR(m->fs_name); > > > + m->fs_name = NULL; > > > + if (err == -ENOMEM) > > > + goto out_err; > > > + else > > > + goto bad; > > > + } > > > } > > > /* damaged */ > > > if (mdsmap_ev >= 9) { > > > @@ -418,6 +426,7 @@ void ceph_mdsmap_destroy(struct ceph_mdsmap *m) > > > kfree(m->m_info); > > > } > > > kfree(m->m_data_pg_pools); > > > + kfree(m->fs_name); > > > kfree(m); > > > } > > > > > > diff --git a/fs/ceph/mdsmap.h b/fs/ceph/mdsmap.h > > > index 1f2171dd01bf..acb0a2a3627a 100644 > > > --- a/fs/ceph/mdsmap.h > > > +++ b/fs/ceph/mdsmap.h > > > @@ -45,6 +45,7 @@ struct ceph_mdsmap { > > > bool m_enabled; > > > bool m_damaged; > > > int m_num_laggy; > > > + char *fs_name; > > > > The ceph_mdsmap structure describes servers in the mds cluster [1]. > > Semantically, I don't see any relation of fs_name with this structure. > > As a result, I don't see the point to keep this pointer in this structure. > > Why the fs_name has been placed in this structure? > > > > Thanks, > > Slava. > > > > > }; > > > > > > static inline struct ceph_entity_addr * > > > > [1] https://elixir.bootlin.com/linux/v6.16/source/fs/ceph/mdsmap.h#L11 > >
