On Wed, 2021-04-21 at 17:22 +0100, Luis Henriques wrote: > Jeff Layton <jlayton@xxxxxxxxxx> writes: > > > The MDS reserves a set of inodes for its own usage, and these should > > never be accessible to clients. Add a new helper to vet a proposed > > inode number against that range, and complain loudly and refuse to > > create or look it up if it's in it. We do need to carve out an exception > > for the root and the lost+found directories. > > > > Also, ensure that the MDS doesn't try to delegate that range to us > > either. Print a warning if it does, and don't save the range in the > > xarray. > > > > URL: https://tracker.ceph.com/issues/49922 > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > Signed-off-by: Xiubo Li<xiubli@xxxxxxxxxx> > > Reviewed-by: Patrick Donnelly <pdonnell@xxxxxxxxxx> > > --- > > fs/ceph/export.c | 8 ++++++++ > > fs/ceph/inode.c | 3 +++ > > fs/ceph/mds_client.c | 7 +++++++ > > fs/ceph/super.h | 24 ++++++++++++++++++++++++ > > include/linux/ceph/ceph_fs.h | 7 ++++--- > > 5 files changed, 46 insertions(+), 3 deletions(-) > > > > v2: allow lookups of lost+found dir inodes > > flesh out and update the CEPH_INO_* definitions > > > > diff --git a/fs/ceph/export.c b/fs/ceph/export.c > > index 17d8c8f4ec89..65540a4429b2 100644 > > --- a/fs/ceph/export.c > > +++ b/fs/ceph/export.c > > @@ -129,6 +129,10 @@ static struct inode *__lookup_inode(struct super_block *sb, u64 ino) > > > > vino.ino = ino; > > vino.snap = CEPH_NOSNAP; > > + > > + if (ceph_vino_is_reserved(vino)) > > + return ERR_PTR(-ESTALE); > > + > > inode = ceph_find_inode(sb, vino); > > if (!inode) { > > struct ceph_mds_request *req; > > @@ -214,6 +218,10 @@ static struct dentry *__snapfh_to_dentry(struct super_block *sb, > > vino.ino = sfh->ino; > > vino.snap = sfh->snapid; > > } > > + > > + if (ceph_vino_is_reserved(vino)) > > + return ERR_PTR(-ESTALE); > > + > > inode = ceph_find_inode(sb, vino); > > if (inode) > > return d_obtain_alias(inode); > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > > index 14a1f7963625..e1c63adb196d 100644 > > --- a/fs/ceph/inode.c > > +++ b/fs/ceph/inode.c > > @@ -56,6 +56,9 @@ struct inode *ceph_get_inode(struct super_block *sb, struct ceph_vino vino) > > { > > struct inode *inode; > > > > + if (ceph_vino_is_reserved(vino)) > > + return ERR_PTR(-EREMOTEIO); > > + > > inode = iget5_locked(sb, (unsigned long)vino.ino, ceph_ino_compare, > > ceph_set_ino_cb, &vino); > > if (!inode) > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > > index 63b53098360c..e5af591d3bd4 100644 > > --- a/fs/ceph/mds_client.c > > +++ b/fs/ceph/mds_client.c > > @@ -440,6 +440,13 @@ static int ceph_parse_deleg_inos(void **p, void *end, > > > > ceph_decode_64_safe(p, end, start, bad); > > ceph_decode_64_safe(p, end, len, bad); > > + > > + /* Don't accept a delegation of system inodes */ > > + if (start < CEPH_INO_SYSTEM_BASE) { > > + pr_warn_ratelimited("ceph: ignoring reserved inode range delegation (start=0x%llx len=0x%llx)\n", > > + start, len); > > + continue; > > + } > > while (len--) { > > int err = xa_insert(&s->s_delegated_inos, ino = start++, > > DELEGATED_INO_AVAILABLE, > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > > index df0851b9240e..f1745403c9b0 100644 > > --- a/fs/ceph/super.h > > +++ b/fs/ceph/super.h > > @@ -529,10 +529,34 @@ static inline int ceph_ino_compare(struct inode *inode, void *data) > > ci->i_vino.snap == pvino->snap; > > } > > > > +/* > > + * The MDS reserves a set of inodes for its own usage. These should never > > + * be accessible by clients, and so the MDS has no reason to ever hand these > > + * out. > > + * > > + * These come from src/mds/mdstypes.h in the ceph sources. > > + */ > > +#define CEPH_MAX_MDS 0x100 > > +#define CEPH_NUM_STRAY 10 > > +#define CEPH_INO_SYSTEM_BASE ((6*CEPH_MAX_MDS) + (CEPH_MAX_MDS * CEPH_NUM_STRAY)) > > + > > +static inline bool ceph_vino_is_reserved(const struct ceph_vino vino) > > +{ > > + if (vino.ino < CEPH_INO_SYSTEM_BASE && > > + vino.ino != CEPH_INO_ROOT && > > + vino.ino != CEPH_INO_LOST_AND_FOUND) { > > + WARN_RATELIMIT(1, "Attempt to access reserved inode number 0x%llx", vino.ino); > > This warning is quite easy to hit with inode 0x3, using generic/013. It > looks like, while looking for the snaprealm to check quotas, we will > eventually get this value from the MDS. So this function should also > return 'true' if ino is CEPH_INO_GLOBAL_SNAPREALM. > I wonder why I'm not seeing this. What mount options are you using? Thanks, -- Jeff Layton <jlayton@xxxxxxxxxx>
