Linux-Security-Module Archive on lore.kernel.org
 help / Atom feed
* Re: [git pull] vfs.git mount.part1
       [not found] <20190104192648.GO2217@ZenIV.linux.org.uk>
@ 2019-01-05 19:31 ` ebiederm
  2019-01-05 20:45   ` Al Viro
  0 siblings, 1 reply; 2+ messages in thread
From: ebiederm @ 2019-01-05 19:31 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, linux-kernel, linux-fsdevel

Al Viro <viro@zeniv.linux.org.uk> writes:

> 	mount API prereqs.  Mostly that's LSM mount options cleanups.
> One trivial conflict in security/selinux/hooks.c, resolved by taking
> the variant from this branch - the method has been split, leaving
> only the part that used to be conditional upon "it's not an internal
> mount" and check has been moved into the caller of the remaining piece.
> The last commit in this pile ("mount_fs: suppress MAC on MS_SUBMOUNT as
> well as MS_KERNMOUNT") is an equivalent of the conflict-creating
> mainline change.
> 	There are several minor fixes in there, but nothing
> earth-shattering (leaks on failure exits, mostly).

Am I completely blind or has this code never been posted for reivew?

I know you stuffed something in linux-next, and I saw quite a bit of
consternation from several of the security folks at the time when
testing on linux-next started reporting failures.

Currently on the linux-security-module list there is an unanswered
question by Casey Schaufler <casey@schaufler-ca.com> of which
tree this work is being performed on.

I know I asked you explicitly when you would be posting this code
for review and did not get an answer.

I know I saw bug activity on your tree right up to the day Linus made
his stable release.  I keeping an eye out in the hopes that there would
be something to review.

Not having had a chance to review this code I can't really comment on
the quality of this code.  What I do know from a glance is that
you have not removed FS_BINARY_MOUNTDATA.  Which is the root cause
of some of the crazy security mount option processing, and is an if
not greater mess than what the security options have been doing with
mount options.

The FS_BINARY_MOUNTDATA flag is only relevant for coda and for nfs
backwards compatiblity.  The FS_BINARY_MOUNTDATA flag is only set on
btrfs to allow calling mount_subtree.

I have a set of patches that is finally reasonablly stable and cleans up
all of the mess in the current internal mount apis that should allow
implementing the new mount api to be much less error prone.  My plan is
to start posting it as soon as the merge window closes.

I would really appreciate it if we can work together on this the next
development cycle, and not try and rush and get things out and merged
without proper review, and care.  That has already led to problems.

Eric


> The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a:
>
>   Linux 4.20-rc1 (2018-11-04 15:37:52 -0800)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git mount.part1
>
> for you to fetch changes up to 718c43038f287e843c2f63d946977de90014cb11:
>
>   mount_fs: suppress MAC on MS_SUBMOUNT as well as MS_KERNMOUNT (2018-12-21 11:51:23 -0500)
>
> ----------------------------------------------------------------
> Al Viro (25):
>       exofs_mount(): fix leaks on failure exits
>       selinux: expand superblock_doinit() calls
>       smack: make smack_parse_opts_str() clean up on failure
>       LSM: lift parsing LSM options into the caller of ->sb_kern_mount()
>       LSM: lift extracting and parsing LSM options into the caller of ->sb_remount()
>       new helper: security_sb_eat_lsm_opts()
>       LSM: split ->sb_set_mnt_opts() out of ->sb_kern_mount()
>       selinux; don't open-code a loop in sb_finish_set_opts()
>       btrfs: sanitize security_mnt_opts use
>       nfs_remount(): don't leak, don't ignore LSM options quietly
>       LSM: turn sb_eat_lsm_opts() into a method
>       selinux: kill selinux_sb_get_mnt_opts()
>       LSM: hide struct security_mnt_opts from any generic code
>       selinux: switch to private struct selinux_mnt_opts
>       smack: switch to private smack_mnt_opts
>       LSM: bury struct security_mnt_opts
>       selinux: new helper - selinux_add_opt()
>       selinux: switch away from match_token()
>       selinux: regularize Opt_... names a bit
>       selinux: rewrite selinux_sb_eat_lsm_opts()
>       LSM: new method: ->sb_add_mnt_opt()
>       smack: take the guts of smack_parse_opts_str() into a new helper
>       smack: get rid of match_token()
>       smack: rewrite smack_sb_eat_lsm_opts()
>       mount_fs: suppress MAC on MS_SUBMOUNT as well as MS_KERNMOUNT
>
> David Howells (2):
>       vfs: Suppress MS_* flag defs within the kernel unless explicitly enabled
>       vfs: Separate changing mount flags full remount
>
>  arch/arc/kernel/setup.c       |   1 +
>  arch/arm/kernel/atags_parse.c |   1 +
>  arch/sh/kernel/setup.c        |   1 +
>  arch/sparc/kernel/setup_32.c  |   1 +
>  arch/sparc/kernel/setup_64.c  |   1 +
>  arch/x86/kernel/setup.c       |   1 +
>  drivers/base/devtmpfs.c       |   1 +
>  fs/btrfs/ctree.h              |   4 -
>  fs/btrfs/super.c              |  82 +----
>  fs/exofs/super.c              |  37 +-
>  fs/namespace.c                | 156 ++++++---
>  fs/nfs/internal.h             |   2 +-
>  fs/nfs/super.c                |  34 +-
>  fs/pnode.c                    |   1 +
>  fs/super.c                    |  24 +-
>  include/linux/lsm_hooks.h     |  17 +-
>  include/linux/mount.h         |   2 +-
>  include/linux/security.h      |  82 +----
>  include/uapi/linux/fs.h       |  56 +--
>  include/uapi/linux/mount.h    |  58 +++
>  init/do_mounts.c              |   1 +
>  init/do_mounts_initrd.c       |   1 +
>  security/apparmor/lsm.c       |   1 +
>  security/apparmor/mount.c     |   1 +
>  security/security.c           |  39 ++-
>  security/selinux/hooks.c      | 799 ++++++++++++++++--------------------------
>  security/smack/smack_lsm.c    | 359 ++++++++-----------
>  security/tomoyo/mount.c       |   1 +
>  28 files changed, 724 insertions(+), 1040 deletions(-)
>  create mode 100644 include/uapi/linux/mount.h

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [git pull] vfs.git mount.part1
  2019-01-05 19:31 ` [git pull] vfs.git mount.part1 ebiederm
@ 2019-01-05 20:45   ` Al Viro
  0 siblings, 0 replies; 2+ messages in thread
From: Al Viro @ 2019-01-05 20:45 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Linus Torvalds, linux-kernel, linux-fsdevel, linux-security-module

On Sat, Jan 05, 2019 at 01:31:21PM -0600, Eric W. Biederman wrote:

> Not having had a chance to review this code I can't really comment on
> the quality of this code.  What I do know from a glance is that
> you have not removed FS_BINARY_MOUNTDATA.  Which is the root cause
> of some of the crazy security mount option processing, and is an if
> not greater mess than what the security options have been doing with
> mount options.
> 
> The FS_BINARY_MOUNTDATA flag is only relevant for coda and for nfs
> backwards compatiblity.  The FS_BINARY_MOUNTDATA flag is only set on
> btrfs to allow calling mount_subtree.

... and thus it can't be killed without having dragged the NFS pile
into the entire thing.

> I have a set of patches that is finally reasonablly stable and cleans up
> all of the mess in the current internal mount apis that should allow
> implementing the new mount api to be much less error prone.

Quick question: how do you deal with the differences in quoting for selinux
options and for everything else?

I've no problem with working with you, now that you've resurfaced.
Fair warning: no promises of accepting your solutions.  Along with
a promise to reject anything that breaks existing setups, which your
earlier proposals did.  With NFS among the victims, IIRC.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190104192648.GO2217@ZenIV.linux.org.uk>
2019-01-05 19:31 ` [git pull] vfs.git mount.part1 ebiederm
2019-01-05 20:45   ` Al Viro

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org linux-security-module@archiver.kernel.org
	public-inbox-index linux-security-module


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/ public-inbox