From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Richard Haines <richard_c_haines@btinternet.com>
Cc: Paul Moore <paul@paul-moore.com>,
Christoph Hellwig <hch@infradead.org>,
Stephen Smalley <sds@tycho.nsa.gov>,
linux-xfs@vger.kernel.org, selinux@vger.kernel.org
Subject: Re: [PATCH 0/1] selinux: Add xfs quota command types
Date: Mon, 24 Feb 2020 20:13:27 -0800 [thread overview]
Message-ID: <20200225041327.GE6740@magnolia> (raw)
In-Reply-To: <5c1f2125a44006d7ff8bda6d9a1075d2177aeaf0.camel@btinternet.com>
On Mon, Feb 24, 2020 at 04:41:17PM +0000, Richard Haines wrote:
> On Sat, 2020-02-22 at 14:49 -0500, Paul Moore wrote:
> > On Thu, Feb 20, 2020 at 11:08 AM Christoph Hellwig <hch@infradead.org
> > > wrote:
> > > On Thu, Feb 20, 2020 at 11:06:10AM -0500, Stephen Smalley wrote:
> > > > > The dquot_* routines are not generic quota code, but a specific
> > > > > implementation used by most non-XFS file systems. So if there
> > > > > is a bug
> > > > > it is that the security call is not in the generic dispatch
> > > > > code.
> > > >
> > > > Hmm...any reason the security hook call couldn't be taken to
> > > > quota_quotaon()?
> > >
> > > I haven't touched the quota code for a while, but yes, the existing
> > > calls should move to the quota_* routines in
> > > fs/quota/quota.c. Note
> > > that you still need to add checks, e.g. for Q_XSETQLIM.
> >
> > Who wanted to submit a patch for this? Christoph were you planning
> > on
> > fixing this? If not, Richard, do you want to give it a try?
> >
>
> I've had a go at this and found I can (almost) get it working. I've
> attached a sample patch just in case anyone is interested.
Almost? Are you talking about the weird looking dentry?
> However if the calls do need to move to fs/quota/quota.c, then I think
> the xfs team are best placed to do this (I've had my playtime).
>
>
> diff --git a/fs/xfs/xfs_quotaops.c b/fs/xfs/xfs_quotaops.c
> index 38669e827..c02cf9a63 100644
> --- a/fs/xfs/xfs_quotaops.c
> +++ b/fs/xfs/xfs_quotaops.c
> @@ -14,7 +14,7 @@
> #include "xfs_trans.h"
> #include "xfs_icache.h"
> #include "xfs_qm.h"
> -
> +#include <linux/security.h>
>
> static void
> xfs_qm_fill_state(
> @@ -161,11 +161,15 @@ xfs_quota_enable(
> unsigned int uflags)
> {
> struct xfs_mount *mp = XFS_M(sb);
> + struct dentry *dentry = mp->m_super->s_root;
>
> if (sb_rdonly(sb))
> return -EROFS;
> if (!XFS_IS_QUOTA_RUNNING(mp))
> return -ENOSYS;
> + /* Emulates dquot_quota_on() Labeled correctly */
> + if (!security_quota_on(dentry))
> + return -EACCES;
>
> return xfs_qm_scall_quotaon(mp, xfs_quota_flags(uflags));
> }
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 2094386af..59855d060 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -39,6 +39,7 @@
> #include <linux/magic.h>
> #include <linux/fs_context.h>
> #include <linux/fs_parser.h>
> +#include <linux/security.h>
>
> static const struct super_operations xfs_super_operations;
>
> @@ -1507,6 +1508,22 @@ xfs_fc_fill_super(
> goto out_unmount;
> }
>
> + /*
> + * Emulates dquot_quota_on_mount() and works, however the dentry
> + * is unlabeled:
> + * allow test_filesystem_t unlabeled_t:dir { quotaon };
> + */
> + struct xfs_mount *mpx = XFS_M(sb);
> + struct dentry *dentry = mpx->m_super->s_root;
> + if (XFS_IS_QUOTA_RUNNING(mp)) {
> + error = security_quota_on(dentry);
Er... I'm not sure if this is doing what you really want it to? Mostly
because I don't know what the quota security hooks are supposed to
activate against? :)
The other three filesystems (ext4/f2fs/reiserfs) keep their quota files
linked off the root directory, and dquot_quota_on_mount looks up the
dentry for a root directory quota file and feeds that into
security_quota_on.
This patch calls it on XFS' root directory itself (quota inodes are not
linked in the directory tree at all on xfs), which I guess is the best
you can do, but might not be what the security hooks is expecting.
<shrug> Apologies for my unfamiliarity.
--D
> + if (error) {
> + dput(sb->s_root);
> + sb->s_root = NULL;
> + goto out_unmount;
> + }
> + }
> +
> return 0;
>
> out_filestream_unmount:
> diff --git a/security/security.c b/security/security.c
> index db7b574c9..ac0cc9872 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -770,6 +770,7 @@ int security_quota_on(struct dentry *dentry)
> {
> return call_int_hook(quota_on, 0, dentry);
> }
> +EXPORT_SYMBOL(security_quota_on);
>
> int security_syslog(int type)
> {
next prev parent reply other threads:[~2020-02-25 4:14 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-20 15:32 [PATCH 0/1] selinux: Add xfs quota command types Richard Haines
2020-02-20 15:32 ` [PATCH 1/1] " Richard Haines
2020-02-20 15:44 ` Christoph Hellwig
2020-02-22 19:47 ` Paul Moore
2020-02-20 15:57 ` [PATCH 0/1] " Darrick J. Wong
2020-02-20 15:59 ` Christoph Hellwig
[not found] ` <2862d0b2-e0a9-149d-16e5-22c3f5f82e9e@tycho.nsa.gov>
2020-02-20 16:08 ` Christoph Hellwig
2020-02-22 19:49 ` Paul Moore
2020-02-24 16:41 ` Richard Haines
2020-02-25 4:13 ` Darrick J. Wong [this message]
2020-02-25 17:54 ` Richard Haines
2020-02-25 21:35 ` Darrick J. Wong
2020-02-26 15:21 ` Stephen Smalley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200225041327.GE6740@magnolia \
--to=darrick.wong@oracle.com \
--cc=hch@infradead.org \
--cc=linux-xfs@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=richard_c_haines@btinternet.com \
--cc=sds@tycho.nsa.gov \
--cc=selinux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).