linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: Paul Moore <paul@paul-moore.com>
Cc: Christoph Hellwig <hch@lst.de>,
	Christian Brauner <brauner@kernel.org>,
	linux-fsdevel@vger.kernel.org, Seth Forshee <sforshee@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	v9fs-developer@lists.sourceforge.net, linux-cifs@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, casey@schaufler-ca.com
Subject: Re: [PATCH v2 00/30] acl: add vfs posix acl api
Date: Tue, 27 Sep 2022 16:37:51 -0700	[thread overview]
Message-ID: <6dcd0fe7-3fc6-9edc-349c-0133ab1f918a@schaufler-ca.com> (raw)
In-Reply-To: <CAHC9VhToUZici98r10UJOQhE48j-58hUheT0P8GE9nC38p=ijQ@mail.gmail.com>

On 9/27/2022 4:24 PM, Paul Moore wrote:
> On Tue, Sep 27, 2022 at 10:13 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 9/27/2022 12:41 AM, Christoph Hellwig wrote:
>>> On Mon, Sep 26, 2022 at 05:22:45PM -0700, Casey Schaufler wrote:
>>>> I suggest that you might focus on the acl/evm interface rather than the entire
>>>> LSM interface. Unless there's a serious plan to make ima/evm into a proper LSM
>>>> I don't see how the breadth of this patch set is appropriate.
>>> Umm. The problem is the historically the Linux xattr interface was
>>> intended for unstructured data, while some of it is very much structured
>>> and requires interpretation by the VFS and associated entities.  So
>>> splitting these out and add proper interface is absolutely the right
>>> thing to do and long overdue (also for other thing like capabilities).
>>> It might make things a little more verbose for LSM, but it fixes a very
>>> real problem.
>> Here's the problem I see. All of the LSMs see xattrs, except for their own,
>> as opaque objects. Introducing LSM hooks to address the data interpretation
>> issues between VFS and EVM, which is not an LSM, adds to an already overlarge
>> and interface. And the "real" users of the interface don't need the new hook.
>> I'm not saying that the ACL doesn't have problems. I'm not saying that the
>> solution you've proposed isn't better than what's there now. I am saying that
>> using LSM as a conduit between VFS and EVM at the expense of the rest of the
>> modules is dubious. A lot of change to LSM for no value to LSM.
> Let's take a step back and look not just at the LSM changes, but the
> patchset as a whole.  Forgive my paraphrasing, but what Christian is
> trying to do here is introduce a proper ACL API in the kernel to
> remove a lot of kludges, special-cases, etc. in the VFS layer,
> enabling better type checking, code abstractions, and all the nice
> things you get when you have nice APIs.  This is admirable work, even
> if it does result in some duplication at the LSM layer (and below).
>
> It is my opinion that the impact to the LSM, both at the LSM layer,
> and in the individual affected LSMs is not significant enough to
> outweigh the other advantages offered by this patchset.

Hey, in the end it's your call. I agree that cleaning up kludgy code
is inherently good. I'm willing to believe that putting further effort
into the patch set to make the LSM aspects cleaner isn't cost effective.
If everyone else thinks this is the right approach, I don't need to
question it further.


      reply	other threads:[~2022-09-27 23:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-26 14:07 [PATCH v2 00/30] acl: add vfs posix acl api Christian Brauner
2022-09-26 14:08 ` [PATCH v2 05/30] cifs: implement get acl method Christian Brauner
2022-09-26 14:08 ` [PATCH v2 06/30] cifs: implement set " Christian Brauner
2022-09-26 14:08 ` [PATCH v2 19/30] ksmbd: use vfs_remove_acl() Christian Brauner
2022-09-26 14:08 ` [PATCH v2 28/30] cifs: use stub posix acl handlers Christian Brauner
2022-09-27  0:22 ` [PATCH v2 00/30] acl: add vfs posix acl api Casey Schaufler
2022-09-27  7:41   ` Christoph Hellwig
2022-09-27  7:59     ` Christian Brauner
2022-09-27 14:11     ` Casey Schaufler
2022-09-27 15:16       ` Seth Forshee
2022-09-27 15:55         ` Casey Schaufler
2022-09-27 23:24       ` Paul Moore
2022-09-27 23:37         ` Casey Schaufler [this message]

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=6dcd0fe7-3fc6-9edc-349c-0133ab1f918a@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=brauner@kernel.org \
    --cc=hch@lst.de \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=sforshee@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=v9fs-developer@lists.sourceforge.net \
    --cc=viro@zeniv.linux.org.uk \
    /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).