linux-kernel - Re: [RFC PATCH 2/2] ceph: quota: fix quota subdir mounts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAM7YA=ry+t3X4ECXZNb51oXtk+5wXSWd6EM46vsZafQ2LL7sg@mail.gmail.com>
Date:   Thu, 7 Mar 2019 22:23:03 +0800
From:   "Yan, Zheng" <ukernel@...il.com>
To:     Luis Henriques <lhenriques@...e.com>
Cc:     "Yan, Zheng" <zyan@...hat.com>, Sage Weil <sage@...hat.com>,
        Ilya Dryomov <idryomov@...il.com>,
        ceph-devel <ceph-devel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Hendrik Peyerl <hpeyerl@...sline.net>
Subject: Re: [RFC PATCH 2/2] ceph: quota: fix quota subdir mounts

On Thu, Mar 7, 2019 at 7:02 PM Luis Henriques <lhenriques@...e.com> wrote:
>
> "Yan, Zheng" <ukernel@...il.com> writes:
>
> > On Thu, Mar 7, 2019 at 2:21 AM Luis Henriques <lhenriques@...e.com> wrote:
> >>
> >> "Yan, Zheng" <ukernel@...il.com> writes:
> >>
> >> > On Sat, Mar 2, 2019 at 3:13 AM Luis Henriques <lhenriques@...e.com> wrote:
> >> >>
> >> >> The CephFS kernel client doesn't enforce quotas that are set in a
> >> >> directory that isn't visible in the mount point.  For example, given the
> >> >> path '/dir1/dir2', if quotas are set in 'dir1' and the mount is done in with
> >> >>
> >> >>   mount -t ceph <server>:<port>:/dir1/ /mnt
> >> >>
> >> >> then the client can't access the 'dir1' inode from the quota realm dir2
> >> >> belongs to.
> >> >>
> >> >> This patch fixes this by simply doing an MDS LOOKUPINO Op and grabbing a
> >> >> reference to it (so that it doesn't disappear again).  This also requires an
> >> >> extra field in ceph_snap_realm so that we know we have to release that
> >> >> reference when destroying the realm.
> >> >>
> >> >
> >> > This may cause circle reference if somehow an inode owned by snaprealm
> >> > get moved into mount subdir (other clients do rename).  how about
> >> > holding these inodes in mds_client?
> >>
> >> Ok, before proceeded any further I wanted to make sure that what you
> >> were suggesting was something like the patch below.  It simply keeps a
> >> list of inodes in ceph_mds_client until the filesystem is umounted,
> >> iput()ing them at that point.
> >>
> > yes,
> >
> >> I'm sure I'm missing another place where the reference should be
> >> dropped, but I couldn't figure it out yet.  It can't be
> >> ceph_destroy_inode; drop_inode_snap_realm is a possibility, but what if
> >> the inode becomes visible in the meantime?  Well, I'll continue thinking
> >> about it.
> >
> > why do you think we need to clean up the references at other place.
> > what problem you encountered.
>
> I'm not really seeing any issue, at least not at the moment.  I believe
> that we could just be holding refs to inodes that may not exist anymore
> in the cluster.  For example, in client 1:
>
>  mkdir -p /mnt/a/b
>  setfattr -n ceph.quota.max_files -v 5 /mnt/a
>
> In client 2 we mount:
>
>  mount <addr>:/a/b /mnt
>
> This client will access the realm and inode for 'a' (adding that inode
> to the ceph_mds_client list), because it has quotas.  If client 1 then
> deletes 'a', client 2 will continue to have a reference to that inode in
> that list.  That's why I thought we should be able to clean up that refs
> list in some other place, although that's probably a big deal, since we
> won't be able to a lot with this mount anyway.
>

Agree, it's not big deal

> Cheers,
> --
> Luis