* [PATCH v3 2/2] prctl: add PR_[GS]ET_KILLABLE
From: Jürg Billeter @ 2018-08-03 14:40 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleg Nesterov, Thomas Gleixner, Eric Biederman, linux-api,
linux-kernel, Jürg Billeter
In-Reply-To: <20180803144021.56920-1-j@bitron.ch>
PR_SET_KILLABLE clears the SIGNAL_UNKILLABLE flag. This allows
CLONE_NEWPID tasks to restore normal signal behavior, opting out of the
special signal protection for init processes. This prctl does not allow
setting the SIGNAL_UNKILLABLE flag, only clearing.
The SIGNAL_UNKILLABLE flag, which is implicitly set for tasks cloned
with CLONE_NEWPID, has the effect of ignoring all signals (from
userspace) if the corresponding handler is set to SIG_DFL. The only
exceptions are SIGKILL and SIGSTOP and they are only accepted if raised
from an ancestor namespace.
SIGINT, SIGQUIT and SIGTSTP are used in job control for ^C, ^\, ^Z.
While a task with the SIGNAL_UNKILLABLE flag could install handlers for
these signals, this is not sufficient to implement a shell that uses
CLONE_NEWPID for child processes:
* As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
itself, it's not possible to implement the stop action in a custom
SIGTSTP handler.
* Many applications do not install handlers for these signals and
thus, job control won't work properly with unmodified applications.
There are other scenarios besides job control in a shell where
applications rely on the default actions as described in signal(7) and
PID isolation may be useful. This new prctl makes the signal protection
for "init" processes optional, without breaking backward compatibility.
Signed-off-by: Jürg Billeter <j@bitron.ch>
---
include/uapi/linux/prctl.h | 4 ++++
kernel/sys.c | 13 +++++++++++++
2 files changed, 17 insertions(+)
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index c0d7ea0bf5b6..92afb63da727 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -219,4 +219,8 @@ struct prctl_mm_map {
# define PR_SPEC_DISABLE (1UL << 2)
# define PR_SPEC_FORCE_DISABLE (1UL << 3)
+/* Control SIGNAL_UNKILLABLE */
+#define PR_GET_KILLABLE 54
+#define PR_SET_KILLABLE 55
+
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..92c9322cfb98 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2484,6 +2484,19 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
return -EINVAL;
error = arch_prctl_spec_ctrl_set(me, arg2, arg3);
break;
+ case PR_GET_KILLABLE:
+ if (arg3 || arg4 || arg5)
+ return -EINVAL;
+ error = put_user(!(me->signal->flags & SIGNAL_UNKILLABLE),
+ (int __user *)arg2);
+ break;
+ case PR_SET_KILLABLE:
+ if (arg2 != 1 || arg3 || arg4 || arg5)
+ return -EINVAL;
+ spin_lock_irq(&me->sighand->siglock);
+ me->signal->flags &= ~SIGNAL_UNKILLABLE;
+ spin_unlock_irq(&me->sighand->siglock);
+ break;
default:
error = -EINVAL;
break;
--
2.18.0
^ permalink raw reply related
* Re: [PATCH 28/33] vfs: syscall: Add fsconfig() for configuring and managing a context [ver #11]
From: Eric W. Biederman @ 2018-08-06 17:28 UTC (permalink / raw)
To: David Howells; +Cc: viro, linux-api, torvalds, linux-fsdevel, linux-kernel
In-Reply-To: <153313723557.13253.9055982745313603422.stgit@warthog.procyon.org.uk>
David Howells <dhowells@redhat.com> writes:
>
> (*) FSCONFIG_CMD_CREATE: Trigger superblock creation.
>
> (*) FSCONFIG_CMD_RECONFIGURE: Trigger superblock reconfiguration.
>
First let me thank you for adding both FSCONFIG_CMD_CREATE and
FSCONFIG_CMD_RECONFIGURE. Unfortunately the implementation is currently
broken. So this patch gets my:
This is broken in two specific ways.
1) FSCONFIG_CMD_RECONFIGURE always returns -EOPNOTSUPPORTED.
So it is useless.
2) FSCONFIG_CMD_CREATE will succeed even if the superblock already
exists and it can not use all of the superblock parameters.
This happens because vfs_get_super will only call fill_super
if the super block is created. Which is reasonable on the face
of it. But it in practice this introduces security problems.
a) Either through reconfiguring a shared super block you did not
realize was shared (as we saw with devpts).
b) Mounting a super block and not honoring it's mount options
because something has already mounted it. As we see today
with proc. Leaving userspace to think the filesystem will behave
one way when in fact it behaves another.
I have already explained this several times, and apparently I have been
ignored. This fundamental usability issue that leads to security
problems.
The only feedback I have had from previous time is that it is ``racy''
to fix the code. But it is only racy in the way that O_EXCL is racy.
You might have to retry in userspace if the mount you want isn't in the
state you expect.
Until this security issue is fixed this entire patchset has my:
Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> +/*
> + * Perform an action on a context.
> + */
> +static int vfs_fsconfig_action(struct fs_context *fc, enum fsconfig_command cmd)
> +{
> + int ret = -EINVAL;
> +
> + switch (cmd) {
> + case FSCONFIG_CMD_CREATE:
> + if (fc->phase != FS_CONTEXT_CREATE_PARAMS)
> + return -EBUSY;
> + fc->phase = FS_CONTEXT_CREATING;
> + ret = vfs_get_tree(fc);
> + if (ret == 0)
> + fc->phase = FS_CONTEXT_AWAITING_MOUNT;
> + else
> + fc->phase = FS_CONTEXT_FAILED;
> + return ret;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
See no support for FSCONFIG_CMD_RECONFIGURE, and no checks to see if
the superblock has already been mounted.
> + ret = mutex_lock_interruptible(&fc->uapi_mutex);
> + if (ret == 0) {
> + switch (cmd) {
> + case FSCONFIG_CMD_CREATE:
> + case FSCONFIG_CMD_RECONFIGURE:
> + ret = vfs_fsconfig_action(fc, cmd);
> + break;
> + default:
> + ret = vfs_fsconfig(fc, ¶m);
> + break;
> + }
> + mutex_unlock(&fc->uapi_mutex);
> + }
> +
Eric
^ permalink raw reply
* Re: [PATCH v4 0/4] seccomp trap to userspace
From: Tycho Andersen @ 2018-08-07 2:44 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, containers, linux-api, Andy Lutomirski,
Oleg Nesterov, Eric W . Biederman, Serge E . Hallyn,
Christian Brauner, Tyler Hicks, Akihiro Suda, Tobin C . Harding
In-Reply-To: <20180621220416.5412-1-tycho@tycho.ws>
Hi all,
Dinesh Subhraveti has claimed that some part of this series might be
patented. While he has not furnished me with anything to confirm this
claim, I'll put this series on hold.
Tycho
On Thu, Jun 21, 2018 at 04:04:12PM -0600, Tycho Andersen wrote:
> Hi all,
>
> Here's v4 of the seccomp trap to userspace series. v3 is here:
> https://lkml.org/lkml/2018/5/31/527
>
> I believe we've addressed the two burning questions I had about v3: 1.
> it seems ok not to use netlink, since there's not a great way to re-use
> the API without a lot of unnecessary code and 2. only having return
> capability for fds seems fine with people. Or at least I haven't heard
> any strong objections.
>
> I've re-worked a bunch of things in this version based on feedback from
> the last series. See patch notes for details. At this point I'm not
> aware of anything that needs to be addressed, but of course that is
> subject to change :)
>
> Tycho
>
> Tycho Andersen (4):
> seccomp: add a return code to trap to userspace
> seccomp: make get_nth_filter available outside of CHECKPOINT_RESTORE
> seccomp: add a way to get a listener fd from ptrace
> seccomp: add support for passing fds via USER_NOTIF
>
> .../userspace-api/seccomp_filter.rst | 79 +++
> arch/Kconfig | 7 +
> include/linux/seccomp.h | 18 +-
> include/uapi/linux/ptrace.h | 2 +
> include/uapi/linux/seccomp.h | 23 +-
> kernel/ptrace.c | 4 +
> kernel/seccomp.c | 491 ++++++++++++++-
> tools/testing/selftests/seccomp/seccomp_bpf.c | 560 +++++++++++++++++-
> 8 files changed, 1172 insertions(+), 12 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v4 0/4] seccomp trap to userspace
From: Andy Lutomirski @ 2018-08-07 2:57 UTC (permalink / raw)
To: Tycho Andersen
Cc: Kees Cook, linux-kernel, containers, linux-api, Oleg Nesterov,
Eric W . Biederman, Serge E . Hallyn, Christian Brauner,
Tyler Hicks, Akihiro Suda, Tobin C . Harding
In-Reply-To: <20180807024442.GA12274@cisco.lan>
> On Aug 6, 2018, at 7:44 PM, Tycho Andersen <tycho@tycho.ws> wrote:
>
> Hi all,
>
> Dinesh Subhraveti has claimed that some part of this series might be
> patented. While he has not furnished me with anything to confirm this
> claim, I'll put this series on hold.
That... is utterly ridiculous. Does LF have a mechanism to figure out wtf and deal with it by, for example, filing for ex parte review.
Every microkernel ever should strongly resemble prior art.
>
> Tycho
>
>> On Thu, Jun 21, 2018 at 04:04:12PM -0600, Tycho Andersen wrote:
>> Hi all,
>>
>> Here's v4 of the seccomp trap to userspace series. v3 is here:
>> https://lkml.org/lkml/2018/5/31/527
>>
>> I believe we've addressed the two burning questions I had about v3: 1.
>> it seems ok not to use netlink, since there's not a great way to re-use
>> the API without a lot of unnecessary code and 2. only having return
>> capability for fds seems fine with people. Or at least I haven't heard
>> any strong objections.
>>
>> I've re-worked a bunch of things in this version based on feedback from
>> the last series. See patch notes for details. At this point I'm not
>> aware of anything that needs to be addressed, but of course that is
>> subject to change :)
>>
>> Tycho
>>
>> Tycho Andersen (4):
>> seccomp: add a return code to trap to userspace
>> seccomp: make get_nth_filter available outside of CHECKPOINT_RESTORE
>> seccomp: add a way to get a listener fd from ptrace
>> seccomp: add support for passing fds via USER_NOTIF
>>
>> .../userspace-api/seccomp_filter.rst | 79 +++
>> arch/Kconfig | 7 +
>> include/linux/seccomp.h | 18 +-
>> include/uapi/linux/ptrace.h | 2 +
>> include/uapi/linux/seccomp.h | 23 +-
>> kernel/ptrace.c | 4 +
>> kernel/seccomp.c | 491 ++++++++++++++-
>> tools/testing/selftests/seccomp/seccomp_bpf.c | 560 +++++++++++++++++-
>> 8 files changed, 1172 insertions(+), 12 deletions(-)
>>
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [PATCH v4 0/4] seccomp trap to userspace
From: Christian Brauner @ 2018-08-07 3:30 UTC (permalink / raw)
To: Tycho Andersen
Cc: Kees Cook, Tobin C . Harding, linux-api, containers, Akihiro Suda,
Oleg Nesterov, linux-kernel, Eric W . Biederman, Andy Lutomirski
In-Reply-To: <20180807024442.GA12274@cisco.lan>
On Mon, Aug 06, 2018 at 08:44:42PM -0600, Tycho Andersen wrote:
> Hi all,
>
> Dinesh Subhraveti has claimed that some part of this series might be
> patented. While he has not furnished me with anything to confirm this
> claim, I'll put this series on hold.
Hey man,
Sorry to hear that your faced with such nonsense, Tycho. This is utter
bullsh*t of course. If you have more details at some point and feel
comfortable doing so it would probably be good to share them here.
Christian
>
> Tycho
>
> On Thu, Jun 21, 2018 at 04:04:12PM -0600, Tycho Andersen wrote:
> > Hi all,
> >
> > Here's v4 of the seccomp trap to userspace series. v3 is here:
> > https://lkml.org/lkml/2018/5/31/527
> >
> > I believe we've addressed the two burning questions I had about v3: 1.
> > it seems ok not to use netlink, since there's not a great way to re-use
> > the API without a lot of unnecessary code and 2. only having return
> > capability for fds seems fine with people. Or at least I haven't heard
> > any strong objections.
> >
> > I've re-worked a bunch of things in this version based on feedback from
> > the last series. See patch notes for details. At this point I'm not
> > aware of anything that needs to be addressed, but of course that is
> > subject to change :)
> >
> > Tycho
> >
> > Tycho Andersen (4):
> > seccomp: add a return code to trap to userspace
> > seccomp: make get_nth_filter available outside of CHECKPOINT_RESTORE
> > seccomp: add a way to get a listener fd from ptrace
> > seccomp: add support for passing fds via USER_NOTIF
> >
> > .../userspace-api/seccomp_filter.rst | 79 +++
> > arch/Kconfig | 7 +
> > include/linux/seccomp.h | 18 +-
> > include/uapi/linux/ptrace.h | 2 +
> > include/uapi/linux/seccomp.h | 23 +-
> > kernel/ptrace.c | 4 +
> > kernel/seccomp.c | 491 ++++++++++++++-
> > tools/testing/selftests/seccomp/seccomp_bpf.c | 560 +++++++++++++++++-
> > 8 files changed, 1172 insertions(+), 12 deletions(-)
> >
> > --
> > 2.17.1
> >
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply
* Re: [PATCH v4 0/4] seccomp trap to userspace
From: Andy Lutomirski @ 2018-08-07 4:19 UTC (permalink / raw)
To: Christian Brauner
Cc: Tycho Andersen, Kees Cook, Tobin C . Harding, Linux API,
Linux Containers, Akihiro Suda, Oleg Nesterov, LKML,
Eric W . Biederman
In-Reply-To: <20180807033043.2esvm5qbhhqvatvy@mailbox.org>
On Mon, Aug 6, 2018 at 8:30 PM, Christian Brauner <christian@brauner.io> wrote:
> On Mon, Aug 06, 2018 at 08:44:42PM -0600, Tycho Andersen wrote:
>> Hi all,
>>
>> Dinesh Subhraveti has claimed that some part of this series might be
>> patented. While he has not furnished me with anything to confirm this
>> claim, I'll put this series on hold.
>
> Hey man,
>
> Sorry to hear that your faced with such nonsense, Tycho. This is utter
> bullsh*t of course. If you have more details at some point and feel
> comfortable doing so it would probably be good to share them here.
IANAL, but I think it would probably *not* be good to share them here.
Patent law is seriously fucked up like that.
--Andy
^ permalink raw reply
* Re: [PATCH v4 0/4] seccomp trap to userspace
From: Christian Brauner @ 2018-08-07 12:23 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Tycho Andersen, Kees Cook, Tobin C . Harding, Linux API,
Linux Containers, Akihiro Suda, Oleg Nesterov, LKML,
Eric W . Biederman
In-Reply-To: <CALCETrVR9r4QsKtwK_+tO7V33G+4r4rkB652rq1A1K2XJvTNrQ@mail.gmail.com>
On Mon, Aug 06, 2018 at 09:19:04PM -0700, Andy Lutomirski wrote:
> On Mon, Aug 6, 2018 at 8:30 PM, Christian Brauner <christian@brauner.io> wrote:
> > On Mon, Aug 06, 2018 at 08:44:42PM -0600, Tycho Andersen wrote:
> >> Hi all,
> >>
> >> Dinesh Subhraveti has claimed that some part of this series might be
> >> patented. While he has not furnished me with anything to confirm this
> >> claim, I'll put this series on hold.
> >
> > Hey man,
> >
> > Sorry to hear that your faced with such nonsense, Tycho. This is utter
> > bullsh*t of course. If you have more details at some point and feel
> > comfortable doing so it would probably be good to share them here.
>
> IANAL, but I think it would probably *not* be good to share them here.
> Patent law is seriously fucked up like that.
**ugh**. Yeah, you're probably right. My thought was that in the face of
publicity this nonesense might go away quickly.
Christian
^ permalink raw reply
* Re: [PATCH] proc: add percpu populated pages count to meminfo
From: Vlastimil Babka @ 2018-08-07 13:18 UTC (permalink / raw)
To: Dennis Zhou, Andrew Morton, Tejun Heo, Johannes Weiner,
Christoph Lameter, Roman Gushchin
Cc: kernel-team, linux-mm, linux-kernel, Linux API
In-Reply-To: <20180807005607.53950-1-dennisszhou@gmail.com>
+CC linux-api
On 08/07/2018 02:56 AM, Dennis Zhou wrote:
> From: "Dennis Zhou (Facebook)" <dennisszhou@gmail.com>
>
> Currently, percpu memory only exposes allocation and utilization
> information via debugfs. This more or less is only really useful for
> understanding the fragmentation and allocation information at a
> per-chunk level with a few global counters. This is also gated behind a
> config. BPF and cgroup, for example, have seen an increase use causing
> increased use of percpu memory. Let's make it easier for someone to
> identify how much memory is being used.
>
> This patch adds the PercpuPopulated stat to meminfo to more easily
> look up how much percpu memory is in use. This new number includes the
> cost for all backing pages and not just insight at the a unit, per
> chunk level. This stat includes only pages used to back the chunks
> themselves excluding metadata. I think excluding metadata is fair
> because the backing memory scales with the number of cpus and can
> quickly outweigh the metadata. It also makes this calculation light.
>
> Signed-off-by: Dennis Zhou <dennisszhou@gmail.com>
Sounds useful and cheap, so why not, I guess.
> ---
> fs/proc/meminfo.c | 2 ++
> include/linux/percpu.h | 2 ++
> mm/percpu.c | 29 +++++++++++++++++++++++++++++
Documentation/filesystems/proc.txt should be updated as well
> 3 files changed, 33 insertions(+)
>
> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> index 2fb04846ed11..ddd5249692e9 100644
> --- a/fs/proc/meminfo.c
> +++ b/fs/proc/meminfo.c
> @@ -7,6 +7,7 @@
> #include <linux/mman.h>
> #include <linux/mmzone.h>
> #include <linux/proc_fs.h>
> +#include <linux/percpu.h>
> #include <linux/quicklist.h>
> #include <linux/seq_file.h>
> #include <linux/swap.h>
> @@ -121,6 +122,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
> (unsigned long)VMALLOC_TOTAL >> 10);
> show_val_kb(m, "VmallocUsed: ", 0ul);
> show_val_kb(m, "VmallocChunk: ", 0ul);
> + show_val_kb(m, "PercpuPopulated:", pcpu_nr_populated_pages());
>
> #ifdef CONFIG_MEMORY_FAILURE
> seq_printf(m, "HardwareCorrupted: %5lu kB\n",
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index 296bbe49d5d1..1c80be42822c 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -149,4 +149,6 @@ extern phys_addr_t per_cpu_ptr_to_phys(void *addr);
> (typeof(type) __percpu *)__alloc_percpu(sizeof(type), \
> __alignof__(type))
>
> +extern int pcpu_nr_populated_pages(void);
> +
> #endif /* __LINUX_PERCPU_H */
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 0b6480979ac7..08a4341f30c5 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -169,6 +169,14 @@ static LIST_HEAD(pcpu_map_extend_chunks);
> */
> int pcpu_nr_empty_pop_pages;
>
> +/*
> + * The number of populated pages in use by the allocator, protected by
> + * pcpu_lock. This number is kept per a unit per chunk (i.e. when a page gets
> + * allocated/deallocated, it is allocated/deallocated in all units of a chunk
> + * and increments/decrements this count by 1).
> + */
> +static int pcpu_nr_populated;
It better be unsigned long, to match others.
> +
> /*
> * Balance work is used to populate or destroy chunks asynchronously. We
> * try to keep the number of populated free pages between
> @@ -1232,6 +1240,7 @@ static void pcpu_chunk_populated(struct pcpu_chunk *chunk, int page_start,
>
> bitmap_set(chunk->populated, page_start, nr);
> chunk->nr_populated += nr;
> + pcpu_nr_populated += nr;
>
> if (!for_alloc) {
> chunk->nr_empty_pop_pages += nr;
> @@ -1260,6 +1269,7 @@ static void pcpu_chunk_depopulated(struct pcpu_chunk *chunk,
> chunk->nr_populated -= nr;
> chunk->nr_empty_pop_pages -= nr;
> pcpu_nr_empty_pop_pages -= nr;
> + pcpu_nr_populated -= nr;
> }
>
> /*
> @@ -2176,6 +2186,9 @@ int __init pcpu_setup_first_chunk(const struct pcpu_alloc_info *ai,
> pcpu_nr_empty_pop_pages = pcpu_first_chunk->nr_empty_pop_pages;
> pcpu_chunk_relocate(pcpu_first_chunk, -1);
>
> + /* include all regions of the first chunk */
> + pcpu_nr_populated += PFN_DOWN(size_sum);
> +
> pcpu_stats_chunk_alloc();
> trace_percpu_create_chunk(base_addr);
>
> @@ -2745,6 +2758,22 @@ void __init setup_per_cpu_areas(void)
>
> #endif /* CONFIG_SMP */
>
> +/*
> + * pcpu_nr_populated_pages - calculate total number of populated backing pages
> + *
> + * This reflects the number of pages populated to back the chunks.
> + * Metadata is excluded in the number exposed in meminfo as the number of
> + * backing pages scales with the number of cpus and can quickly outweigh the
> + * memory used for metadata. It also keeps this calculation nice and simple.
> + *
> + * RETURNS:
> + * Total number of populated backing pages in use by the allocator.
> + */
> +int pcpu_nr_populated_pages(void)
Also unsigned long please.
Thanks,
Vlastimil
> +{
> + return pcpu_nr_populated * pcpu_nr_units;
> +}
> +
> /*
> * Percpu allocator is initialized early during boot when neither slab or
> * workqueue is available. Plug async management until everything is up
>
^ permalink raw reply
* Re: [PATCH v4 0/4] seccomp trap to userspace
From: James Bottomley @ 2018-08-07 14:34 UTC (permalink / raw)
To: Tycho Andersen, Kees Cook
Cc: Tobin C . Harding, linux-api, containers, Akihiro Suda,
Oleg Nesterov, linux-kernel, Eric W . Biederman,
Christian Brauner, Andy Lutomirski
In-Reply-To: <20180807024442.GA12274@cisco.lan>
On Mon, 2018-08-06 at 20:44 -0600, Tycho Andersen wrote:
> Hi all,
>
> Dinesh Subhraveti has claimed that some part of this series might be
> patented. While he has not furnished me with anything to confirm this
> claim, I'll put this series on hold.
Just on a general policy for handling things like this.
Firstly, never discuss specifics on a mailing list. Patents are a
really tricky area where advice in confidence is needed and lack of
confidentiality usually only benefits the patent holder.
Secondly, vague threats (like this) can be safely ignored. Thanks to
the terms of the GPL and the specific actions of OIN, we legitimately
have licences to a vast pool of patents, so we should assume for every
vague threat of the existence of a patent that we do legitimately
possess a licence until the threat is made specific.
Finally, if the vague threat becomes specific (which means citation of
patent by number and claim) you need to engage the resources we have at
our disposal to investigate. Likely it will be OIN resources that do
this investigation, but the way to begin is to start with your
corporate counsel for contributions on behalf of a company. If they're
on behalf of you as an individual, you can begin with Counsel which the
Linux Foundation will provide.
James
^ permalink raw reply
* Re: [PATCH] proc: add percpu populated pages count to meminfo
From: Dennis Zhou @ 2018-08-07 15:03 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, Tejun Heo, Johannes Weiner, Christoph Lameter,
Roman Gushchin, kernel-team, linux-mm, linux-kernel, Linux API
In-Reply-To: <3b792413-184b-20b1-9d90-9e69f0df8cc4@suse.cz>
Hi Vlastimil,
On Tue, Aug 07, 2018 at 03:18:31PM +0200, Vlastimil Babka wrote:
>
> Documentation/filesystems/proc.txt should be updated as well
>
Will do.
> >
> > +/*
> > + * The number of populated pages in use by the allocator, protected by
> > + * pcpu_lock. This number is kept per a unit per chunk (i.e. when a page gets
> > + * allocated/deallocated, it is allocated/deallocated in all units of a chunk
> > + * and increments/decrements this count by 1).
> > + */
> > +static int pcpu_nr_populated;
>
> It better be unsigned long, to match others.
>
Yeah that makes sense. I've changed this for v2.
> > +/*
> > + * pcpu_nr_populated_pages - calculate total number of populated backing pages
> > + *
> > + * This reflects the number of pages populated to back the chunks.
> > + * Metadata is excluded in the number exposed in meminfo as the number of
> > + * backing pages scales with the number of cpus and can quickly outweigh the
> > + * memory used for metadata. It also keeps this calculation nice and simple.
> > + *
> > + * RETURNS:
> > + * Total number of populated backing pages in use by the allocator.
> > + */
> > +int pcpu_nr_populated_pages(void)
>
> Also unsigned long please.
>
Changed for v2.
Thanks,
Dennis
^ permalink raw reply
* [PATCH v2] proc: add percpu populated pages count to meminfo
From: Dennis Zhou @ 2018-08-07 18:47 UTC (permalink / raw)
To: Andrew Morton, Tejun Heo, Johannes Weiner, Christoph Lameter,
Roman Gushchin, Vlastimil Babka
Cc: kernel-team, linux-mm, linux-kernel, Linux API,
Dennis Zhou (Facebook)
From: "Dennis Zhou (Facebook)" <dennisszhou@gmail.com>
Currently, percpu memory only exposes allocation and utilization
information via debugfs. This more or less is only really useful for
understanding the fragmentation and allocation information at a
per-chunk level with a few global counters. This is also gated behind a
config. BPF and cgroup, for example, have seen an increase use causing
increased use of percpu memory. Let's make it easier for someone to
identify how much memory is being used.
This patch adds the "Percpu" stat to meminfo to more easily look up how
much percpu memory is in use. This number includes the cost for all
allocated backing pages and not just isnight at the a unit, per chunk
level. Metadata is excluded. I think excluding metadata is fair because
the backing memory scales with the numbere of cpus and can quickly
outweigh the metadata. It also makes this calculation light.
Signed-off-by: Dennis Zhou <dennisszhou@gmail.com>
---
Documentation/filesystems/proc.txt | 3 +++
fs/proc/meminfo.c | 2 ++
include/linux/percpu.h | 2 ++
mm/percpu.c | 29 +++++++++++++++++++++++++++++
4 files changed, 36 insertions(+)
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 520f6a84cf50..490f803be1ec 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -870,6 +870,7 @@ Committed_AS: 100056 kB
VmallocTotal: 112216 kB
VmallocUsed: 428 kB
VmallocChunk: 111088 kB
+Percpu: 62080 kB
AnonHugePages: 49152 kB
ShmemHugePages: 0 kB
ShmemPmdMapped: 0 kB
@@ -959,6 +960,8 @@ Committed_AS: The amount of memory presently allocated on the system.
VmallocTotal: total size of vmalloc memory area
VmallocUsed: amount of vmalloc area which is used
VmallocChunk: largest contiguous block of vmalloc area which is free
+ Percpu: Memory allocated to the percpu allocator used to back percpu
+ allocations. This stat excludes the cost of metadata.
..............................................................................
diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index 2fb04846ed11..edda898714eb 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -7,6 +7,7 @@
#include <linux/mman.h>
#include <linux/mmzone.h>
#include <linux/proc_fs.h>
+#include <linux/percpu.h>
#include <linux/quicklist.h>
#include <linux/seq_file.h>
#include <linux/swap.h>
@@ -121,6 +122,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
(unsigned long)VMALLOC_TOTAL >> 10);
show_val_kb(m, "VmallocUsed: ", 0ul);
show_val_kb(m, "VmallocChunk: ", 0ul);
+ show_val_kb(m, "Percpu: ", pcpu_nr_pages());
#ifdef CONFIG_MEMORY_FAILURE
seq_printf(m, "HardwareCorrupted: %5lu kB\n",
diff --git a/include/linux/percpu.h b/include/linux/percpu.h
index 296bbe49d5d1..70b7123f38c7 100644
--- a/include/linux/percpu.h
+++ b/include/linux/percpu.h
@@ -149,4 +149,6 @@ extern phys_addr_t per_cpu_ptr_to_phys(void *addr);
(typeof(type) __percpu *)__alloc_percpu(sizeof(type), \
__alignof__(type))
+extern unsigned long pcpu_nr_pages(void);
+
#endif /* __LINUX_PERCPU_H */
diff --git a/mm/percpu.c b/mm/percpu.c
index 0b6480979ac7..a749d4d96e3e 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -169,6 +169,14 @@ static LIST_HEAD(pcpu_map_extend_chunks);
*/
int pcpu_nr_empty_pop_pages;
+/*
+ * The number of populated pages in use by the allocator, protected by
+ * pcpu_lock. This number is kept per a unit per chunk (i.e. when a page gets
+ * allocated/deallocated, it is allocated/deallocated in all units of a chunk
+ * and increments/decrements this count by 1).
+ */
+static unsigned long pcpu_nr_populated;
+
/*
* Balance work is used to populate or destroy chunks asynchronously. We
* try to keep the number of populated free pages between
@@ -1232,6 +1240,7 @@ static void pcpu_chunk_populated(struct pcpu_chunk *chunk, int page_start,
bitmap_set(chunk->populated, page_start, nr);
chunk->nr_populated += nr;
+ pcpu_nr_populated += nr;
if (!for_alloc) {
chunk->nr_empty_pop_pages += nr;
@@ -1260,6 +1269,7 @@ static void pcpu_chunk_depopulated(struct pcpu_chunk *chunk,
chunk->nr_populated -= nr;
chunk->nr_empty_pop_pages -= nr;
pcpu_nr_empty_pop_pages -= nr;
+ pcpu_nr_populated -= nr;
}
/*
@@ -2176,6 +2186,9 @@ int __init pcpu_setup_first_chunk(const struct pcpu_alloc_info *ai,
pcpu_nr_empty_pop_pages = pcpu_first_chunk->nr_empty_pop_pages;
pcpu_chunk_relocate(pcpu_first_chunk, -1);
+ /* include all regions of the first chunk */
+ pcpu_nr_populated += PFN_DOWN(size_sum);
+
pcpu_stats_chunk_alloc();
trace_percpu_create_chunk(base_addr);
@@ -2745,6 +2758,22 @@ void __init setup_per_cpu_areas(void)
#endif /* CONFIG_SMP */
+/*
+ * pcpu_nr_pages - calculate total number of populated backing pages
+ *
+ * This reflects the number of pages populated to back chunks. Metadata is
+ * excluded in the number exposed in meminfo as the number of backing pages
+ * scales with the number of cpus and can quickly outweigh the memory used for
+ * metadata. It also keeps this calculation nice and simple.
+ *
+ * RETURNS:
+ * Total number of populated backing pages in use by the allocator.
+ */
+unsigned long pcpu_nr_pages(void)
+{
+ return pcpu_nr_populated * pcpu_nr_units;
+}
+
/*
* Percpu allocator is initialized early during boot when neither slab or
* workqueue is available. Plug async management until everything is up
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2] proc: add percpu populated pages count to meminfo
From: Tejun Heo @ 2018-08-07 18:51 UTC (permalink / raw)
To: Dennis Zhou
Cc: Andrew Morton, Johannes Weiner, Christoph Lameter, Roman Gushchin,
Vlastimil Babka, kernel-team, linux-mm, linux-kernel, Linux API
In-Reply-To: <20180807184723.74919-1-dennisszhou@gmail.com>
Hello, Dennis.
On Tue, Aug 07, 2018 at 11:47:23AM -0700, Dennis Zhou wrote:
> From: "Dennis Zhou (Facebook)" <dennisszhou@gmail.com>
>
> Currently, percpu memory only exposes allocation and utilization
> information via debugfs. This more or less is only really useful for
> understanding the fragmentation and allocation information at a
> per-chunk level with a few global counters. This is also gated behind a
> config. BPF and cgroup, for example, have seen an increase use causing
> increased use of percpu memory. Let's make it easier for someone to
> identify how much memory is being used.
>
> This patch adds the "Percpu" stat to meminfo to more easily look up how
> much percpu memory is in use. This number includes the cost for all
> allocated backing pages and not just isnight at the a unit, per chunk
> level. Metadata is excluded. I think excluding metadata is fair because
> the backing memory scales with the numbere of cpus and can quickly
> outweigh the metadata. It also makes this calculation light.
>
> Signed-off-by: Dennis Zhou <dennisszhou@gmail.com>
Acked-by: Tejun Heo <tj@kernel.org>
Andrew, if this looks good, can you please route this?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v2] proc: add percpu populated pages count to meminfo
From: Roman Gushchin @ 2018-08-07 20:10 UTC (permalink / raw)
To: Dennis Zhou
Cc: Andrew Morton, Tejun Heo, Johannes Weiner, Christoph Lameter,
Vlastimil Babka, kernel-team, linux-mm, linux-kernel, Linux API
In-Reply-To: <20180807184723.74919-1-dennisszhou@gmail.com>
On Tue, Aug 07, 2018 at 11:47:23AM -0700, Dennis Zhou wrote:
> From: "Dennis Zhou (Facebook)" <dennisszhou@gmail.com>
>
> Currently, percpu memory only exposes allocation and utilization
> information via debugfs. This more or less is only really useful for
> understanding the fragmentation and allocation information at a
> per-chunk level with a few global counters. This is also gated behind a
> config. BPF and cgroup, for example, have seen an increase use causing
> increased use of percpu memory. Let's make it easier for someone to
> identify how much memory is being used.
>
> This patch adds the "Percpu" stat to meminfo to more easily look up how
> much percpu memory is in use. This number includes the cost for all
> allocated backing pages and not just isnight at the a unit, per chunk
> level. Metadata is excluded. I think excluding metadata is fair because
> the backing memory scales with the numbere of cpus and can quickly
> outweigh the metadata. It also makes this calculation light.
>
> Signed-off-by: Dennis Zhou <dennisszhou@gmail.com>
Acked-by: Roman Gushchin <guro@fb.com>
It's super useful! I've seen hosts in production which have
tens and hundreds on megabytes in per-cpu memory, and with
vmalloc counters being defined to 0, it's really hard
to notice and track down.
Thanks, Dennis!
^ permalink raw reply
* [RESEND][PATCH v5 0/2] vfs: better dedupe permission check
From: Mark Fasheh @ 2018-08-07 21:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Al Viro, linux-fsdevel, linux-api, Michael Kerrisk, linux-btrfs,
linux-xfs, Darrick J . Wong, Adam Borowski, David Sterba,
Mark Fasheh
Hi Andrew,
Could I please have these patches upstreamed or at least put in a tree for
more public testing? They've hit fsdevel a few times now, I have links to
the discussions in the change log below.
The following patches fix a couple of issues with the permission check
we do in vfs_dedupe_file_range().
The first patch expands our check to allow dedupe of a file if the
user owns it or otherwise would be allowed to write to it.
Current behavior is that we'll allow dedupe only if:
- the user is an admin (root)
- the user has the file open for write
This makes it impossible for a user to dedupe their own file set
unless they do it as root, or ensure that all files have write
permission. There's a couple of duperemove bugs open for this:
https://github.com/markfasheh/duperemove/issues/129
https://github.com/markfasheh/duperemove/issues/86
The other problem we have is also related to forcing the user to open
target files for write - A process trying to exec a file currently
being deduped gets ETXTBUSY. The answer (as above) is to allow them to
open the targets ro - root can already do this. There was a patch from
Adam Borowski to fix this back in 2016:
https://lkml.org/lkml/2016/7/17/130
which I have incorporated into my changes.
The 2nd patch fixes our return code for permission denied to be
EPERM. For some reason we're returning EINVAL - I think that's
probably my fault. At any rate, we need to be returning something
descriptive of the actual problem, otherwise callers see EINVAL and
can't really make a valid determination of what's gone wrong.
This has also popped up in duperemove, mostly in the form of cryptic
error messages. Because this is a code returned to userspace, I did
check the other users of extent-same that I could find. Both 'bees'
and 'rust-btrfs' do the same as duperemove and simply report the error
(as they should).
Lastly, I have an update to the fi_deduperange manpage to reflect these
changes. That patch is attached below.
Please apply.
git pull https://github.com/markfasheh/linux dedupe-perms
Thanks,
--Mark
Changes from V4 to V5:
- Rebase and retest on 4.18-rc8
- Place updated manpage patch below, CC linux-api
- V4 discussion: https://patchwork.kernel.org/patch/10530365/
Changes from V3 to V4:
- Add a patch (below) to ioctl_fideduperange.2 explaining our
changes. I will send this patch once the kernel update is
accepted. Thanks to Darrick Wong for this suggestion.
- V3 discussion: https://www.spinics.net/lists/linux-btrfs/msg79135.html
Changes from V2 to V3:
- Return bool from allow_file_dedupe
- V2 discussion: https://www.spinics.net/lists/linux-btrfs/msg78421.html
Changes from V1 to V2:
- Add inode_permission check as suggested by Adam Borowski
- V1 discussion: https://marc.info/?l=linux-xfs&m=152606684017965&w=2
From: Mark Fasheh <mfasheh@suse.de>
[PATCH] ioctl_fideduperange.2: clarify permission requirements
dedupe permission checks were recently relaxed - update our man page to
reflect those changes.
Signed-off-by: Mark Fasheh <mfasheh@suse.de>
---
man2/ioctl_fideduperange.2 | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/man2/ioctl_fideduperange.2 b/man2/ioctl_fideduperange.2
index 84d20a276..4040ee064 100644
--- a/man2/ioctl_fideduperange.2
+++ b/man2/ioctl_fideduperange.2
@@ -105,9 +105,12 @@ The field
must be zero.
During the call,
.IR src_fd
-must be open for reading and
+must be open for reading.
.IR dest_fd
-must be open for writing.
+can be open for writing, or reading.
+If
+.IR dest_fd
+is open for reading, the user must have write access to the file.
The combined size of the struct
.IR file_dedupe_range
and the struct
@@ -185,8 +188,8 @@ This can appear if the filesystem does not support deduplicating either file
descriptor, or if either file descriptor refers to special inodes.
.TP
.B EPERM
-.IR dest_fd
-is immutable.
+This will be returned if the user lacks permission to dedupe the file referenced by
+.IR dest_fd .
.TP
.B ETXTBSY
One of the files is a swap file.
--
2.15.1
^ permalink raw reply related
* [PATCH v5 1/2] vfs: allow dedupe of user owned read-only files
From: Mark Fasheh @ 2018-08-07 21:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Al Viro, linux-fsdevel, linux-api, Michael Kerrisk, linux-btrfs,
linux-xfs, Darrick J . Wong, Adam Borowski, David Sterba,
Mark Fasheh
In-Reply-To: <20180807214949.7714-1-mfasheh@suse.de>
The permission check in vfs_dedupe_file_range() is too coarse - We
only allow dedupe of the destination file if the user is root, or
they have the file open for write.
This effectively limits a non-root user from deduping their own read-only
files. In addition, the write file descriptor that the user is forced to
hold open can prevent execution of files. As file data during a dedupe
does not change, the behavior is unexpected and this has caused a number of
issue reports. For an example, see:
https://github.com/markfasheh/duperemove/issues/129
So change the check so we allow dedupe on the target if:
- the root or admin is asking for it
- the process has write access
- the owner of the file is asking for the dedupe
- the process could get write access
That way users can open read-only and still get dedupe.
Signed-off-by: Mark Fasheh <mfasheh@suse.de>
Acked-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/read_write.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index e83bd9744b5d..71e9077f8bc1 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1964,6 +1964,20 @@ int vfs_dedupe_file_range_compare(struct inode *src, loff_t srcoff,
}
EXPORT_SYMBOL(vfs_dedupe_file_range_compare);
+/* Check whether we are allowed to dedupe the destination file */
+static bool allow_file_dedupe(struct file *file)
+{
+ if (capable(CAP_SYS_ADMIN))
+ return true;
+ if (file->f_mode & FMODE_WRITE)
+ return true;
+ if (uid_eq(current_fsuid(), file_inode(file)->i_uid))
+ return true;
+ if (!inode_permission(file_inode(file), MAY_WRITE))
+ return true;
+ return false;
+}
+
int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
{
struct file_dedupe_range_info *info;
@@ -1972,7 +1986,6 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
u64 len;
int i;
int ret;
- bool is_admin = capable(CAP_SYS_ADMIN);
u16 count = same->dest_count;
struct file *dst_file;
loff_t dst_off;
@@ -2036,7 +2049,7 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
if (info->reserved) {
info->status = -EINVAL;
- } else if (!(is_admin || (dst_file->f_mode & FMODE_WRITE))) {
+ } else if (!allow_file_dedupe(dst_file)) {
info->status = -EINVAL;
} else if (file->f_path.mnt != dst_file->f_path.mnt) {
info->status = -EXDEV;
--
2.15.1
^ permalink raw reply related
* [PATCH v5 2/2] vfs: dedupe should return EPERM if permission is not granted
From: Mark Fasheh @ 2018-08-07 21:49 UTC (permalink / raw)
To: Andrew Morton
Cc: Al Viro, linux-fsdevel, linux-api, Michael Kerrisk, linux-btrfs,
linux-xfs, Darrick J . Wong, Adam Borowski, David Sterba,
Mark Fasheh
In-Reply-To: <20180807214949.7714-1-mfasheh@suse.de>
Right now we return EINVAL if a process does not have permission to dedupe a
file. This was an oversight on my part. EPERM gives a true description of
the nature of our error, and EINVAL is already used for the case that the
filesystem does not support dedupe.
Signed-off-by: Mark Fasheh <mfasheh@suse.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Acked-by: David Sterba <dsterba@suse.com>
---
fs/read_write.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 71e9077f8bc1..7188982e2733 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -2050,7 +2050,7 @@ int vfs_dedupe_file_range(struct file *file, struct file_dedupe_range *same)
if (info->reserved) {
info->status = -EINVAL;
} else if (!allow_file_dedupe(dst_file)) {
- info->status = -EINVAL;
+ info->status = -EPERM;
} else if (file->f_path.mnt != dst_file->f_path.mnt) {
info->status = -EXDEV;
} else if (S_ISDIR(dst->i_mode)) {
--
2.15.1
^ permalink raw reply related
* Re: [RESEND][PATCH v5 0/2] vfs: better dedupe permission check
From: Adam Borowski @ 2018-08-07 22:21 UTC (permalink / raw)
To: Mark Fasheh
Cc: Andrew Morton, Al Viro, linux-fsdevel, linux-api, Michael Kerrisk,
linux-btrfs, linux-xfs, Darrick J . Wong, David Sterba
In-Reply-To: <20180807214949.7714-1-mfasheh@suse.de>
On Tue, Aug 07, 2018 at 02:49:47PM -0700, Mark Fasheh wrote:
> Hi Andrew,
>
> Could I please have these patches upstreamed or at least put in a tree for
> more public testing? They've hit fsdevel a few times now, I have links to
> the discussions in the change log below.
> The first patch expands our check to allow dedupe of a file if the
> user owns it or otherwise would be allowed to write to it.
[...]
> The other problem we have is also related to forcing the user to open
> target files for write - A process trying to exec a file currently
> being deduped gets ETXTBUSY. The answer (as above) is to allow them to
> open the targets ro - root can already do this. There was a patch from
> Adam Borowski to fix this back in 2016
> The 2nd patch fixes our return code for permission denied to be
> EPERM. For some reason we're returning EINVAL - I think that's
> probably my fault. At any rate, we need to be returning something
> descriptive of the actual problem, otherwise callers see EINVAL and
> can't really make a valid determination of what's gone wrong.
Note that the counterpart of these two patches for BTRFS_IOC_DEFRAG, which
fixes the same issues, is included in btrfs' for-next, slated for 4.19.
While technically dedupe and defrag are independent, there would be somewhat
less confusion if both behave the same in the same kernel version.
Thus, it'd be nice if you would consider taking this. Should be safe:
even the permission check is paranoid.
Meow!
--
⢀⣴⠾⠻⢶⣦⠀ So a Hungarian gypsy mountainman, lumberjack by day job,
⣾⠁⢰⠒⠀⣿⡁ brigand by, uhm, hobby, invented a dish: goulash on potato
⢿⡄⠘⠷⠚⠋⠀ pancakes. Then the Polish couldn't decide which of his
⠈⠳⣄⠀⠀⠀⠀ adjectives to use for the dish's name.
^ permalink raw reply
* Re: [PATCH v2] proc: add percpu populated pages count to meminfo
From: David Rientjes @ 2018-08-08 0:12 UTC (permalink / raw)
To: Dennis Zhou
Cc: Andrew Morton, Tejun Heo, Johannes Weiner, Christoph Lameter,
Roman Gushchin, Vlastimil Babka, kernel-team, linux-mm,
linux-kernel, Linux API
In-Reply-To: <20180807184723.74919-1-dennisszhou@gmail.com>
On Tue, 7 Aug 2018, Dennis Zhou wrote:
> From: "Dennis Zhou (Facebook)" <dennisszhou@gmail.com>
>
> Currently, percpu memory only exposes allocation and utilization
> information via debugfs. This more or less is only really useful for
> understanding the fragmentation and allocation information at a
> per-chunk level with a few global counters. This is also gated behind a
> config. BPF and cgroup, for example, have seen an increase use causing
> increased use of percpu memory. Let's make it easier for someone to
> identify how much memory is being used.
>
> This patch adds the "Percpu" stat to meminfo to more easily look up how
> much percpu memory is in use. This number includes the cost for all
> allocated backing pages and not just isnight at the a unit, per chunk
> level. Metadata is excluded. I think excluding metadata is fair because
> the backing memory scales with the numbere of cpus and can quickly
> outweigh the metadata. It also makes this calculation light.
>
> Signed-off-by: Dennis Zhou <dennisszhou@gmail.com>
Acked-by: David Rientjes <rientjes@google.com>
^ permalink raw reply
* Re: [PATCH v2] proc: add percpu populated pages count to meminfo
From: Vlastimil Babka @ 2018-08-08 7:53 UTC (permalink / raw)
To: Dennis Zhou, Andrew Morton, Tejun Heo, Johannes Weiner,
Christoph Lameter, Roman Gushchin
Cc: kernel-team, linux-mm, linux-kernel, Linux API
In-Reply-To: <20180807184723.74919-1-dennisszhou@gmail.com>
On 08/07/2018 08:47 PM, Dennis Zhou wrote:
> From: "Dennis Zhou (Facebook)" <dennisszhou@gmail.com>
>
> Currently, percpu memory only exposes allocation and utilization
> information via debugfs. This more or less is only really useful for
> understanding the fragmentation and allocation information at a
> per-chunk level with a few global counters. This is also gated behind a
> config. BPF and cgroup, for example, have seen an increase use causing
> increased use of percpu memory. Let's make it easier for someone to
> identify how much memory is being used.
>
> This patch adds the "Percpu" stat to meminfo to more easily look up how
> much percpu memory is in use. This number includes the cost for all
> allocated backing pages and not just isnight at the a unit, per chunk
> level. Metadata is excluded. I think excluding metadata is fair because
> the backing memory scales with the numbere of cpus and can quickly
> outweigh the metadata. It also makes this calculation light.
>
> Signed-off-by: Dennis Zhou <dennisszhou@gmail.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Thanks.
^ permalink raw reply
* Re: [PATCH 28/33] vfs: syscall: Add fsconfig() for configuring and managing a context [ver #11]
From: David Howells @ 2018-08-09 14:14 UTC (permalink / raw)
To: Eric W. Biederman
Cc: dhowells, viro, linux-api, torvalds, linux-fsdevel, linux-kernel
In-Reply-To: <87in4n9zg0.fsf@xmission.com>
Eric W. Biederman <ebiederm@xmission.com> wrote:
> First let me thank you for adding both FSCONFIG_CMD_CREATE and
> FSCONFIG_CMD_RECONFIGURE. Unfortunately the implementation is currently
> broken. So this patch gets my:
>
> This is broken in two specific ways.
> 1) FSCONFIG_CMD_RECONFIGURE always returns -EOPNOTSUPPORTED.
> So it is useless.
This isn't broken, just not completely implemented. I would like to get the
core VFS framework upstream so that filesystems can start being converted.
However, since you asked nicely, here's a patch that adds the reconfiguration
bit.
David
---
vfs: Use fs_context for reconfig and implement FSCONFIG_CMD_RECONFIGURE
Implement fs_context-based reconfiguration by the following means:
(1) Provide two more internal fs_context purposes: umount that actually
reconfigures to R/O and emergency reconfiguration to R/O. This tells
the filesystem if it the context hasn't been fully initialised.
(2) Track which bits in sb_mask have been changed in addition to what
they've been changed to.
(3) Move ->reconfigure() from the superblock ops to the fs_context ops.
This makes (4) possible. The ->remount_fs() superblock op is
obsolete.
(4) Provide a legacy wrapper for ->reconfigure().
(5) Make a do_umount() that's unmounting a root allocate an fs_context on
the stack and use that to reconfigure to R/O.
(6) Make do_emergency_remount_callback() allocate an fs_context on the
stack and use that to reconfigurate to R/O.
(6) Make do_remount() unconditionally use an fs_context to invoke
do_remount_sb().
(7) Only pass in a filesystem context to do_remount_sb(). This, along
with (4), allows the function to be simplified.
(8) Pass errors back from mount_single() if reconfiguration fails. We
might want this behaviour to be conditional, depending on which mount
API was used.
(9) Delete security_sb_remount().
(10) Rename do_remount_sb() to reconfigure_super().
Notes:
(1) do_remount() can't make use of vfs_reconfigure_sb() if the former
changes the mount attributes atomically or if the latter doesn't do so
at all.
However, since I think Al wants us to move towards separating
superblock reconfiguration from mountpoint reconfiguration, there may
not be a need to do this atomically.
(2) mount_single() probably shouldn't reconfigure an already existing
superblock if it's supposed to be creating a new one, but rather it
(or, rather, the filesystem) should compare the parameters and either
return the live superblock if the params are the same or return an
error if not.
However, this probably needs to be contingent on the mount API or
something so as not to break userspace.
(3) I should add something like an FSOPEN_EXCL flag to tell sget_fc() to
fail if the superblock already exists at all and an
FSOPEN_FAIL_ON_MISMATCH flag to explicitly say that we *don't* want a
superblock with different parameters. The implication of providing
neither flag is that we are happy to accept a superblock from the same
source but with different parameters.
Signed-off-by: David Howells <dhowells@redhat.com>
---
Documentation/filesystems/mount_api.txt | 56 +++++++-------
fs/afs/mntpt.c | 2
fs/afs/super.c | 2
fs/fs_context.c | 46 ++++++-----
fs/fsopen.c | 84 +++++++++++++++++++--
fs/hugetlbfs/inode.c | 2
fs/internal.h | 9 +-
fs/kernfs/mount.c | 5 -
fs/libfs.c | 3
fs/namespace.c | 87 +++++++++-------------
fs/nfs/super.c | 2
fs/proc/inode.c | 1
fs/proc/internal.h | 1
fs/proc/root.c | 6 +
fs/super.c | 125 ++++++++++++++++++++------------
include/linux/fs.h | 1
include/linux/fs_context.h | 8 +-
include/linux/kernfs.h | 1
include/linux/lsm_hooks.h | 9 --
include/linux/security.h | 6 -
ipc/mqueue.c | 2
kernel/cgroup/cgroup.c | 2
kernel/cgroup/cpuset.c | 3
security/security.c | 5 -
security/selinux/hooks.c | 1
25 files changed, 283 insertions(+), 186 deletions(-)
diff --git a/Documentation/filesystems/mount_api.txt b/Documentation/filesystems/mount_api.txt
index 5fec78eed4f4..35cc5c7a5008 100644
--- a/Documentation/filesystems/mount_api.txt
+++ b/Documentation/filesystems/mount_api.txt
@@ -55,16 +55,6 @@ purposes - otherwise it will be NULL.
Note that security initialisation is done *after* the filesystem is called so
that the namespaces may be adjusted first.
-And the super_operations struct gains one field:
-
- int (*reconfigure)(struct super_block *, struct fs_context *);
-
-This shadows the ->reconfigure() operation and takes a prepared filesystem
-context instead of the mount flags and data page. It may modify the sb_flags
-in the context for the caller to pick up.
-
-[NOTE] reconfigure is intended as a replacement for remount_fs.
-
======================
THE FILESYSTEM CONTEXT
@@ -86,6 +76,7 @@ context. This is represented by the fs_context structure:
void *security;
void *s_fs_info;
unsigned int sb_flags;
+ unsigned int sb_flags_mask;
enum fs_context_purpose purpose:8;
bool sloppy:1;
bool silent:1;
@@ -150,8 +141,9 @@ The fs_context fields are as follows:
sget_fc(). This can be used to distinguish superblocks.
(*) unsigned int sb_flags
+ (*) unsigned int sb_flags_mask
- This holds the SB_* flags to be set in super_block::s_flags.
+ Which bits SB_* flags are to be set/cleared in super_block::s_flags.
(*) enum fs_context_purpose
@@ -162,6 +154,10 @@ The fs_context fields are as follows:
FS_CONTEXT_FOR_KERNEL_MOUNT, -- New superblock for kernel-internal mount
FS_CONTEXT_FOR_SUBMOUNT -- New automatic submount of extant mount
FS_CONTEXT_FOR_RECONFIGURE -- Change an existing mount
+ FS_CONTEXT_FOR_UMOUNT -- Reconfigure to R/O for umount()
+ FS_CONTEXT_FOR_EMERGENCY_RO -- Emergency reconfigure to R/O
+
+ In the last two cases, ->init_fs_context() will not have been called.
(*) bool sloppy
(*) bool silent
@@ -174,8 +170,8 @@ The fs_context fields are as follows:
[NOTE] silent is probably redundant with sb_flags & SB_SILENT.
-The mount context is created by calling vfs_new_fs_context(), vfs_sb_reconfig()
-or vfs_dup_fs_context() and is destroyed with put_fs_context(). Note that the
+The mount context is created by calling vfs_new_fs_context() or
+vfs_dup_fs_context() and is destroyed with put_fs_context(). Note that the
structure is not refcounted.
VFS, security and filesystem mount options are set individually with
@@ -206,6 +202,7 @@ The filesystem context points to a table of operations:
size_t data_size);
int (*validate)(struct fs_context *fc);
int (*get_tree)(struct fs_context *fc);
+ int (*reconfigure)(struct fs_context *fc);
};
These operations are invoked by the various stages of the mount procedure to
@@ -278,6 +275,18 @@ manage the filesystem context. They are as follows:
The phase on a userspace-driven context will be set to only allow this to
be called once on any particular context.
+ (*) int (*reconfigure)(struct fs_context *fc);
+
+ Called to effect reconfiguration of a superblock using information stored
+ in the filesystem context. It may detach any resources it desires from
+ the filesystem context and transfer them to the superblock. The
+ superblock can be found from fc->root->d_sb.
+
+ On success it should return 0. In the case of an error, it should return
+ a negative error code.
+
+ [NOTE] reconfigure is intended as a replacement for remount_fs.
+
===========================
FILESYSTEM CONTEXT SECURITY
@@ -358,24 +367,19 @@ one for destroying a context:
(*) struct fs_context *vfs_new_fs_context(struct file_system_type *fs_type,
struct dentry *reference,
unsigned int sb_flags,
+ unsigned int sb_flags_mask,
enum fs_context_purpose purpose);
Create a filesystem context for a given filesystem type and purpose. This
- allocates the filesystem context, sets the flags, initialises the security
- and calls fs_type->init_fs_context() to initialise the filesystem private
- data.
+ allocates the filesystem context, sets the superblock flags, initialises
+ the security and calls fs_type->init_fs_context() to initialise the
+ filesystem private data.
reference can be NULL or it may indicate the root dentry of a superblock
- that is going to be reconfigured (FS_CONTEXT_FOR_RECONFIGURE) or the
- automount point that triggered a submount (FS_CONTEXT_FOR_SUBMOUNT). This
- is provided as a source of namespace information.
-
- (*) struct fs_context *vfs_sb_reconfig(struct vfsmount *mnt,
- unsigned int sb_flags);
-
- Create a filesystem context from the same filesystem as an extant mount
- and initialise the mount parameters from the superblock underlying that
- mount. This is for use by superblock parameter reconfiguration.
+ that is going to be reconfigured (FS_CONTEXT_FOR_RECONFIGURE,
+ FS_CONTEXT_FOR_UMOUNT or FS_CONTEXT_FOR_EMERGENCY_RO) or the automount
+ point that triggered a submount (FS_CONTEXT_FOR_SUBMOUNT). This is
+ provided as a source of namespace information.
(*) struct fs_context *vfs_dup_fs_context(struct fs_context *src_fc);
diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index c8a7f05b9f12..16ee515b51c9 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -147,7 +147,7 @@ static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt)
BUG_ON(!d_inode(mntpt));
- fc = vfs_new_fs_context(&afs_fs_type, mntpt, 0,
+ fc = vfs_new_fs_context(&afs_fs_type, mntpt, 0, 0,
FS_CONTEXT_FOR_SUBMOUNT);
if (IS_ERR(fc))
return ERR_CAST(fc);
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 7c97836e7937..43cf1a6a4bf7 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -634,7 +634,7 @@ static int afs_init_fs_context(struct fs_context *fc, struct dentry *reference)
}
break;
- case FS_CONTEXT_FOR_RECONFIGURE:
+ default:
break;
}
diff --git a/fs/fs_context.c b/fs/fs_context.c
index a6597a2fbf2b..2e9a88f41071 100644
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -106,12 +106,14 @@ static int vfs_parse_sb_flag(struct fs_context *fc, const char *key)
token = lookup_constant(common_set_sb_flag, key, 0);
if (token) {
fc->sb_flags |= token;
+ fc->sb_flags_mask |= token;
return 1;
}
token = lookup_constant(common_clear_sb_flag, key, 0);
if (token) {
fc->sb_flags &= ~token;
+ fc->sb_flags_mask |= token;
return 1;
}
@@ -240,6 +242,7 @@ EXPORT_SYMBOL(generic_parse_monolithic);
* @fs_type: The filesystem type.
* @reference: The dentry from which this one derives (or NULL)
* @sb_flags: Filesystem/superblock flags (SB_*)
+ * @sb_flags_mask: Applicable members of @sb_flags
* @purpose: The purpose that this configuration shall be used for.
*
* Open a filesystem and create a mount context. The mount context is
@@ -250,6 +253,7 @@ EXPORT_SYMBOL(generic_parse_monolithic);
struct fs_context *vfs_new_fs_context(struct file_system_type *fs_type,
struct dentry *reference,
unsigned int sb_flags,
+ unsigned int sb_flags_mask,
enum fs_context_purpose purpose)
{
int (*init_fs_context)(struct fs_context *, struct dentry *);
@@ -262,6 +266,7 @@ struct fs_context *vfs_new_fs_context(struct file_system_type *fs_type,
fc->purpose = purpose;
fc->sb_flags = sb_flags;
+ fc->sb_flags_mask = sb_flags_mask;
fc->fs_type = get_filesystem(fs_type);
fc->cred = get_current_cred();
@@ -280,6 +285,8 @@ struct fs_context *vfs_new_fs_context(struct file_system_type *fs_type,
fc->net_ns = get_net(current->nsproxy->net_ns);
break;
case FS_CONTEXT_FOR_RECONFIGURE:
+ case FS_CONTEXT_FOR_UMOUNT:
+ case FS_CONTEXT_FOR_EMERGENCY_RO:
/* We don't pin any namespaces as the superblock's
* subscriptions cannot be changed at this point.
*/
@@ -314,28 +321,6 @@ struct fs_context *vfs_new_fs_context(struct file_system_type *fs_type,
}
EXPORT_SYMBOL(vfs_new_fs_context);
-/**
- * vfs_sb_reconfig - Create a filesystem context for remount/reconfiguration
- * @mountpoint: The mountpoint to open
- * @sb_flags: Filesystem/superblock flags (SB_*)
- *
- * Open a mounted filesystem and create a filesystem context such that a
- * remount can be effected.
- */
-struct fs_context *vfs_sb_reconfig(struct path *mountpoint,
- unsigned int sb_flags)
-{
- struct fs_context *fc;
-
- fc = vfs_new_fs_context(mountpoint->dentry->d_sb->s_type,
- mountpoint->dentry,
- sb_flags, FS_CONTEXT_FOR_RECONFIGURE);
- if (IS_ERR(fc))
- return fc;
-
- return fc;
-}
-
/**
* vfs_dup_fc_config: Duplicate a filesytem context.
* @src_fc: The context to copy.
@@ -754,6 +739,22 @@ static int legacy_get_tree(struct fs_context *fc)
return ret;
}
+/*
+ * Handle remount.
+ */
+static int legacy_reconfigure(struct fs_context *fc)
+{
+ struct legacy_fs_context *ctx = fc->fs_private;
+ struct super_block *sb = fc->root->d_sb;
+
+ if (!sb->s_op->remount_fs)
+ return 0;
+
+ return sb->s_op->remount_fs(sb, &fc->sb_flags,
+ ctx ? ctx->legacy_data : NULL,
+ ctx ? ctx->data_size : 0);
+}
+
const struct fs_context_operations legacy_fs_context_ops = {
.free = legacy_fs_context_free,
.dup = legacy_fs_context_dup,
@@ -761,6 +762,7 @@ const struct fs_context_operations legacy_fs_context_ops = {
.parse_monolithic = legacy_parse_monolithic,
.validate = legacy_validate,
.get_tree = legacy_get_tree,
+ .reconfigure = legacy_reconfigure,
};
/*
diff --git a/fs/fsopen.c b/fs/fsopen.c
index e79bb5b085d6..9ead9220e2cb 100644
--- a/fs/fsopen.c
+++ b/fs/fsopen.c
@@ -137,7 +137,7 @@ SYSCALL_DEFINE2(fsopen, const char __user *, _fs_name, unsigned int, flags)
if (!fs_type)
return -ENODEV;
- fc = vfs_new_fs_context(fs_type, NULL, 0, FS_CONTEXT_FOR_USER_MOUNT);
+ fc = vfs_new_fs_context(fs_type, NULL, 0, 0, FS_CONTEXT_FOR_USER_MOUNT);
put_filesystem(fs_type);
if (IS_ERR(fc))
return PTR_ERR(fc);
@@ -185,12 +185,8 @@ SYSCALL_DEFINE3(fspick, int, dfd, const char __user *, path, unsigned int, flags
if (ret < 0)
goto err;
- ret = -EOPNOTSUPP;
- if (!target.dentry->d_sb->s_op->reconfigure)
- goto err_path;
-
fc = vfs_new_fs_context(target.dentry->d_sb->s_type, target.dentry,
- 0, FS_CONTEXT_FOR_RECONFIGURE);
+ 0, 0, FS_CONTEXT_FOR_RECONFIGURE);
if (IS_ERR(fc)) {
ret = PTR_ERR(fc);
goto err_path;
@@ -255,6 +251,58 @@ static int vfs_fsconfig(struct fs_context *fc, struct fs_parameter *param)
return vfs_parse_fs_param(fc, param);
}
+/*
+ * Reconfigure a superblock.
+ */
+int vfs_reconfigure_sb(struct fs_context *fc)
+{
+ struct super_block *sb = fc->root->d_sb;
+ int ret;
+
+ if (fc->ops->validate) {
+ ret = fc->ops->validate(fc);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = security_fs_context_validate(fc);
+ if (ret)
+ return ret;
+
+ if (!ns_capable(sb->s_user_ns, CAP_SYS_ADMIN))
+ return -EPERM;
+
+ down_write(&sb->s_umount);
+ ret = reconfigure_super(fc);
+ up_write(&sb->s_umount);
+ return ret;
+}
+
+/*
+ * Clean up a context after performing an action on it and put it into a state
+ * from where it can be used to reconfigure a superblock.
+ */
+void vfs_clean_context(struct fs_context *fc)
+{
+ if (fc->ops && fc->ops->free)
+ fc->ops->free(fc);
+ fc->need_free = false;
+ fc->fs_private = NULL;
+ fc->s_fs_info = NULL;
+ fc->sb_flags = 0;
+ fc->sloppy = false;
+ fc->silent = false;
+ security_fs_context_free(fc);
+ fc->security = NULL;
+ kfree(fc->subtype);
+ fc->subtype = NULL;
+ kfree(fc->source);
+ fc->source = NULL;
+
+ fc->purpose = FS_CONTEXT_FOR_RECONFIGURE;
+ fc->phase = FS_CONTEXT_AWAITING_RECONF;
+}
+
/*
* Perform an action on a context.
*/
@@ -274,6 +322,30 @@ static int vfs_fsconfig_action(struct fs_context *fc, enum fsconfig_command cmd)
fc->phase = FS_CONTEXT_FAILED;
return ret;
+ case FSCONFIG_CMD_RECONFIGURE:
+ if (fc->phase == FS_CONTEXT_AWAITING_RECONF) {
+ /* This is probably pointless, since no changes have
+ * been proposed.
+ */
+ if (fc->fs_type->init_fs_context) {
+ ret = fc->fs_type->init_fs_context(fc, fc->root);
+ if (ret < 0) {
+ fc->phase = FS_CONTEXT_FAILED;
+ return ret;
+ }
+ fc->need_free = true;
+ }
+ fc->phase = FS_CONTEXT_RECONF_PARAMS;
+ }
+
+ fc->phase = FS_CONTEXT_RECONFIGURING;
+ ret = vfs_reconfigure_sb(fc);
+ if (ret == 0)
+ vfs_clean_context(fc);
+ else
+ fc->phase = FS_CONTEXT_FAILED;
+ return ret;
+
default:
return -EOPNOTSUPP;
}
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index e2378c8ce7e0..c09a1cd4fa5a 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1510,7 +1510,7 @@ static struct vfsmount *__init mount_one_hugetlbfs(struct hstate *h)
struct vfsmount *mnt;
int ret;
- fc = vfs_new_fs_context(&hugetlbfs_fs_type, NULL, 0,
+ fc = vfs_new_fs_context(&hugetlbfs_fs_type, NULL, 0, 0,
FS_CONTEXT_FOR_KERNEL_MOUNT);
if (IS_ERR(fc)) {
ret = PTR_ERR(fc);
diff --git a/fs/internal.h b/fs/internal.h
index e5bdfc52b9a1..9c7dd6f12f35 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -54,6 +54,11 @@ extern void __init chrdev_init(void);
*/
extern const struct fs_context_operations legacy_fs_context_ops;
+/*
+ * fsopen.c
+ */
+extern void vfs_clean_context(struct fs_context *fc);
+
/*
* namei.c
*/
@@ -77,6 +82,7 @@ int do_linkat(int olddfd, const char __user *oldname, int newdfd,
*/
extern void *copy_mount_options(const void __user *);
extern char *copy_mount_string(const void __user *);
+extern int parse_monolithic_mount_data(struct fs_context *, void *, size_t);
extern struct vfsmount *lookup_mnt(const struct path *);
extern int finish_automount(struct vfsmount *, struct path *);
@@ -106,8 +112,7 @@ extern struct file *get_empty_filp(void);
/*
* super.c
*/
-extern int do_remount_sb(struct super_block *, int, void *, size_t, int,
- struct fs_context *);
+extern int reconfigure_super(struct fs_context *);
extern bool trylock_super(struct super_block *sb);
extern struct super_block *user_get_super(dev_t);
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index b568e6c5e063..ec14dc76fe89 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -23,9 +23,9 @@
struct kmem_cache *kernfs_node_cache;
-static int kernfs_sop_reconfigure(struct super_block *sb, struct fs_context *fc)
+int kernfs_reconfigure(struct fs_context *fc)
{
- struct kernfs_root *root = kernfs_info(sb)->root;
+ struct kernfs_root *root = kernfs_info(fc->root->d_sb)->root;
struct kernfs_syscall_ops *scops = root->syscall_ops;
if (scops && scops->reconfigure)
@@ -75,7 +75,6 @@ const struct super_operations kernfs_sops = {
.drop_inode = generic_delete_inode,
.evict_inode = kernfs_evict_inode,
- .reconfigure = kernfs_sop_reconfigure,
.show_options = kernfs_sop_show_options,
.show_path = kernfs_sop_show_path,
.get_fsinfo = kernfs_sop_get_fsinfo,
diff --git a/fs/libfs.c b/fs/libfs.c
index d9a5d883dc3f..b1744c071ab0 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -583,7 +583,8 @@ int simple_pin_fs(struct file_system_type *type, struct vfsmount **mount, int *c
if (unlikely(!*mount)) {
spin_unlock(&pin_fs_lock);
- fc = vfs_new_fs_context(type, NULL, 0, FS_CONTEXT_FOR_KERNEL_MOUNT);
+ fc = vfs_new_fs_context(type, NULL, 0, 0,
+ FS_CONTEXT_FOR_KERNEL_MOUNT);
if (IS_ERR(fc))
return PTR_ERR(fc);
diff --git a/fs/namespace.c b/fs/namespace.c
index e34e3fd064b0..47aea9542bf1 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1479,6 +1479,25 @@ static void umount_tree(struct mount *mnt, enum umount_tree_flags how)
static void shrink_submounts(struct mount *mnt);
+static int do_umount_root(struct super_block *sb)
+{
+ int ret = 0;
+ struct fs_context fc = {
+ .purpose = FS_CONTEXT_FOR_UMOUNT,
+ .fs_type = sb->s_type,
+ .root = sb->s_root,
+ .sb_flags = SB_RDONLY,
+ .sb_flags_mask = SB_RDONLY,
+ };
+
+ down_write(&sb->s_umount);
+ if (!sb_rdonly(sb))
+ /* Might want to call ->init_fs_context(). */
+ ret = reconfigure_super(&fc);
+ up_write(&sb->s_umount);
+ return ret;
+}
+
static int do_umount(struct mount *mnt, int flags)
{
struct super_block *sb = mnt->mnt.mnt_sb;
@@ -1544,11 +1563,7 @@ static int do_umount(struct mount *mnt, int flags)
*/
if (!ns_capable(sb->s_user_ns, CAP_SYS_ADMIN))
return -EPERM;
- down_write(&sb->s_umount);
- if (!sb_rdonly(sb))
- retval = do_remount_sb(sb, SB_RDONLY, NULL, 0, 0, NULL);
- up_write(&sb->s_umount);
- return retval;
+ return do_umount_root(sb);
}
namespace_lock();
@@ -2394,7 +2409,7 @@ static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags)
/*
* Parse the monolithic page of mount data given to sys_mount().
*/
-static int parse_monolithic_mount_data(struct fs_context *fc, void *data, size_t data_size)
+int parse_monolithic_mount_data(struct fs_context *fc, void *data, size_t data_size)
{
int (*monolithic_mount_data)(struct fs_context *, void *, size_t);
@@ -2417,7 +2432,6 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
int err;
struct super_block *sb = path->mnt->mnt_sb;
struct mount *mnt = real_mount(path->mnt);
- struct file_system_type *type = sb->s_type;
if (!check_mnt(mnt))
return -EINVAL;
@@ -2428,41 +2442,34 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
if (!can_change_locked_flags(mnt, mnt_flags))
return -EPERM;
- if (type->init_fs_context) {
- fc = vfs_sb_reconfig(path, sb_flags);
- if (IS_ERR(fc))
- return PTR_ERR(fc);
+ fc = vfs_new_fs_context(path->dentry->d_sb->s_type,
+ path->dentry, sb_flags, MS_RMT_MASK,
+ FS_CONTEXT_FOR_RECONFIGURE);
- err = parse_monolithic_mount_data(fc, data, data_size);
+ err = parse_monolithic_mount_data(fc, data, data_size);
+ if (err < 0)
+ goto err_fc;
+
+ if (fc->ops->validate) {
+ err = fc->ops->validate(fc);
if (err < 0)
goto err_fc;
-
- if (fc->ops->validate) {
- err = fc->ops->validate(fc);
- if (err < 0)
- goto err_fc;
- }
-
- err = security_fs_context_validate(fc);
- if (err)
- return err;
- } else {
- err = security_sb_remount(sb, data, data_size);
- if (err)
- return err;
}
+ err = security_fs_context_validate(fc);
+ if (err)
+ return err;
+
down_write(&sb->s_umount);
err = -EPERM;
if (ns_capable(sb->s_user_ns, CAP_SYS_ADMIN)) {
- err = do_remount_sb(sb, sb_flags, data, data_size, 0, fc);
+ err = reconfigure_super(fc);
if (!err)
set_mount_attributes(mnt, mnt_flags);
}
up_write(&sb->s_umount);
err_fc:
- if (fc)
- put_fs_context(fc);
+ put_fs_context(fc);
return err;
}
@@ -2667,7 +2674,7 @@ static int do_new_mount(struct path *mountpoint, const char *fstype,
if (!fs_type)
goto out;
- fc = vfs_new_fs_context(fs_type, NULL, sb_flags,
+ fc = vfs_new_fs_context(fs_type, NULL, sb_flags, sb_flags,
FS_CONTEXT_FOR_USER_MOUNT);
put_filesystem(fs_type);
if (IS_ERR(fc)) {
@@ -3294,7 +3301,7 @@ struct vfsmount *vfs_kern_mount(struct file_system_type *type,
if (!type)
return ERR_PTR(-EINVAL);
- fc = vfs_new_fs_context(type, NULL, sb_flags,
+ fc = vfs_new_fs_context(type, NULL, sb_flags, sb_flags,
sb_flags & SB_KERNMOUNT ?
FS_CONTEXT_FOR_KERNEL_MOUNT :
FS_CONTEXT_FOR_USER_MOUNT);
@@ -3436,23 +3443,7 @@ SYSCALL_DEFINE3(fsmount, int, fs_fd, unsigned int, flags, unsigned int, ms_flags
* do any memory allocation or anything like that at this point as we
* don't want to have to handle any errors incurred.
*/
- if (fc->ops && fc->ops->free)
- fc->ops->free(fc);
- fc->need_free = false;
- fc->fs_private = NULL;
- fc->s_fs_info = NULL;
- fc->sb_flags = 0;
- fc->sloppy = false;
- fc->silent = false;
- security_fs_context_free(fc);
- fc->security = NULL;
- kfree(fc->subtype);
- fc->subtype = NULL;
- kfree(fc->source);
- fc->source = NULL;
-
- fc->purpose = FS_CONTEXT_FOR_RECONFIGURE;
- fc->phase = FS_CONTEXT_AWAITING_RECONF;
+ vfs_clean_context(fc);
/* Attach to an apparent O_PATH fd with a note that we need to unmount
* it, not just simply put it.
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index b5f27d6999e5..9a4eec0ef20a 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2296,7 +2296,7 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data, size_t data_size
/*
* noac is a special case. It implies -o sync, but that's not
- * necessarily reflected in the mtab options. do_remount_sb
+ * necessarily reflected in the mtab options. reconfigure_super
* will clear SB_SYNCHRONOUS if -o sync wasn't specified in the
* remount options, so we have to explicitly reset it.
*/
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 38155bec4a54..8d6f46558fa4 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -127,7 +127,6 @@ const struct super_operations proc_sops = {
.drop_inode = generic_delete_inode,
.evict_inode = proc_evict_inode,
.statfs = simple_statfs,
- .reconfigure = proc_reconfigure,
.show_options = proc_show_options,
};
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index ea8c5468eafc..75a225688a4c 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -273,7 +273,6 @@ static inline void proc_tty_init(void) {}
extern struct proc_dir_entry proc_root;
extern void proc_self_init(void);
-extern int proc_reconfigure(struct super_block *, struct fs_context *);
/*
* task_[no]mmu.c
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 1d6e5bfa30cc..64aa32155432 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -148,8 +148,9 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
return proc_setup_thread_self(s);
}
-int proc_reconfigure(struct super_block *sb, struct fs_context *fc)
+static int proc_reconfigure(struct fs_context *fc)
{
+ struct super_block *sb = fc->root->d_sb;
struct pid_namespace *pid = sb->s_fs_info;
sync_filesystem(sb);
@@ -180,6 +181,7 @@ static const struct fs_context_operations proc_fs_context_ops = {
.free = proc_fs_context_free,
.parse_param = proc_parse_param,
.get_tree = proc_get_tree,
+ .reconfigure = proc_reconfigure,
};
static int proc_init_fs_context(struct fs_context *fc, struct dentry *reference)
@@ -310,7 +312,7 @@ int pid_ns_prepare_proc(struct pid_namespace *ns)
struct vfsmount *mnt;
int ret;
- fc = vfs_new_fs_context(&proc_fs_type, NULL, 0,
+ fc = vfs_new_fs_context(&proc_fs_type, NULL, 0, 0,
FS_CONTEXT_FOR_KERNEL_MOUNT);
if (IS_ERR(fc))
return PTR_ERR(fc);
diff --git a/fs/super.c b/fs/super.c
index 321fbc244570..3f3389e94344 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -920,32 +920,30 @@ struct super_block *user_get_super(dev_t dev)
}
/**
- * do_remount_sb - asks filesystem to change mount options.
- * @sb: superblock in question
- * @sb_flags: revised superblock flags
- * @data: the rest of options
- * @data_size: The size of the data
- * @force: whether or not to force the change
- * @fc: the superblock config for filesystems that support it
- * (NULL if called from emergency or umount)
+ * reconfigure_super - asks filesystem to change superblock parameters
+ * @fc: the superblock and configuration
*
- * Alters the mount options of a mounted file system.
+ * Alters the configuration parameters of a live superblock.
*/
-int do_remount_sb(struct super_block *sb, int sb_flags, void *data,
- size_t data_size, int force, struct fs_context *fc)
+int reconfigure_super(struct fs_context *fc)
{
+ struct super_block *sb = fc->root->d_sb;
int retval;
- int remount_ro;
+ int remount_ro = false;
+ if (fc->sb_flags_mask & ~MS_RMT_MASK)
+ return -EINVAL;
if (sb->s_writers.frozen != SB_UNFROZEN)
return -EBUSY;
+ if (fc->sb_flags_mask & SB_RDONLY) {
#ifdef CONFIG_BLOCK
- if (!(sb_flags & SB_RDONLY) && bdev_read_only(sb->s_bdev))
- return -EACCES;
+ if (!(fc->sb_flags & SB_RDONLY) && bdev_read_only(sb->s_bdev))
+ return -EACCES;
#endif
- remount_ro = (sb_flags & SB_RDONLY) && !sb_rdonly(sb);
+ remount_ro = (fc->sb_flags & SB_RDONLY) && !sb_rdonly(sb);
+ }
if (remount_ro) {
if (!hlist_empty(&sb->s_pins)) {
@@ -956,15 +954,16 @@ int do_remount_sb(struct super_block *sb, int sb_flags, void *data,
return 0;
if (sb->s_writers.frozen != SB_UNFROZEN)
return -EBUSY;
- remount_ro = (sb_flags & SB_RDONLY) && !sb_rdonly(sb);
+ remount_ro = !sb_rdonly(sb);
}
}
shrink_dcache_sb(sb);
- /* If we are remounting RDONLY and current sb is read/write,
- make sure there are no rw files opened */
+ /* If we are reconfiguring to RDONLY and current sb is read/write,
+ * make sure there are no files open for writing.
+ */
if (remount_ro) {
- if (force) {
+ if (fc->purpose == FS_CONTEXT_FOR_EMERGENCY_RO) {
sb->s_readonly_remount = 1;
smp_wmb();
} else {
@@ -974,29 +973,21 @@ int do_remount_sb(struct super_block *sb, int sb_flags, void *data,
}
}
- if (sb->s_op->reconfigure ||
- sb->s_op->remount_fs) {
- if (sb->s_op->reconfigure) {
- retval = sb->s_op->reconfigure(sb, fc);
- if (fc)
- sb_flags = fc->sb_flags;
- else
- sb_flags = sb->s_flags;
- if (retval == 0)
- security_sb_reconfigure(fc);
+ if (fc->ops->reconfigure) {
+ retval = fc->ops->reconfigure(fc);
+ if (retval == 0) {
+ security_sb_reconfigure(fc);
} else {
- retval = sb->s_op->remount_fs(sb, &sb_flags,
- data, data_size);
- }
- if (retval) {
- if (!force)
+ if (fc->purpose != FS_CONTEXT_FOR_EMERGENCY_RO)
goto cancel_readonly;
/* If forced remount, go ahead despite any errors */
WARN(1, "forced remount of a %s fs returned %i\n",
sb->s_type->name, retval);
}
}
- sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (sb_flags & MS_RMT_MASK);
+
+ WRITE_ONCE(sb->s_flags, ((sb->s_flags & ~fc->sb_flags_mask) |
+ (fc->sb_flags & fc->sb_flags_mask)));
/* Needs to be ordered wrt mnt_is_readonly() */
smp_wmb();
sb->s_readonly_remount = 0;
@@ -1020,14 +1011,22 @@ int do_remount_sb(struct super_block *sb, int sb_flags, void *data,
static void do_emergency_remount_callback(struct super_block *sb)
{
+ struct fs_context fc = {
+ .purpose = FS_CONTEXT_FOR_EMERGENCY_RO,
+ .fs_type = sb->s_type,
+ .root = sb->s_root,
+ .sb_flags = SB_RDONLY,
+ .sb_flags_mask = SB_RDONLY,
+ };
+
down_write(&sb->s_umount);
if (sb->s_root && sb->s_bdev && (sb->s_flags & SB_BORN) &&
- !sb_rdonly(sb)) {
+ !sb_rdonly(sb))
+ /* Might want to call ->init_fs_context(). */
/*
* What lock protects sb->s_flags??
*/
- do_remount_sb(sb, SB_RDONLY, NULL, 0, 1, NULL);
- }
+ reconfigure_super(&fc);
up_write(&sb->s_umount);
}
@@ -1416,6 +1415,42 @@ struct dentry *mount_nodev(struct file_system_type *fs_type,
}
EXPORT_SYMBOL(mount_nodev);
+static int reconfigure_single(struct super_block *s,
+ int flags, void *data, size_t data_size)
+{
+ struct fs_context *fc;
+ int ret;
+
+ /* The caller really need to be passing fc down into mount_single(),
+ * then a chunk of this can be removed. Better yet, reconfiguration
+ * shouldn't happen, but rather the second mount should be rejected if
+ * the parameters are not compatible.
+ */
+ fc = vfs_new_fs_context(s->s_type, s->s_root, flags, MS_RMT_MASK,
+ FS_CONTEXT_FOR_RECONFIGURE);
+ if (IS_ERR(fc))
+ return PTR_ERR(fc);
+
+ ret = parse_monolithic_mount_data(fc, data, data_size);
+ if (ret < 0)
+ goto out;
+
+ if (fc->ops->validate) {
+ ret = fc->ops->validate(fc);
+ if (ret < 0)
+ goto out;
+ }
+
+ ret = security_fs_context_validate(fc);
+ if (ret)
+ goto out;
+
+ ret = reconfigure_super(fc);
+out:
+ put_fs_context(fc);
+ return ret;
+}
+
static int compare_single(struct super_block *s, void *p)
{
return 1;
@@ -1433,15 +1468,19 @@ struct dentry *mount_single(struct file_system_type *fs_type,
return ERR_CAST(s);
if (!s->s_root) {
error = fill_super(s, data, data_size, flags & SB_SILENT ? 1 : 0);
- if (error) {
- deactivate_locked_super(s);
- return ERR_PTR(error);
- }
+ if (error)
+ goto error;
s->s_flags |= SB_ACTIVE;
} else {
- do_remount_sb(s, flags, data, data_size, 0, NULL);
+ error = reconfigure_single(s, flags, data, data_size);
+ if (error)
+ goto error;
}
return dget(s->s_root);
+
+error:
+ deactivate_locked_super(s);
+ return ERR_PTR(error);
}
EXPORT_SYMBOL(mount_single);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 053d53861995..1300d77efe96 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1853,7 +1853,6 @@ struct super_operations {
int (*statfs) (struct dentry *, struct kstatfs *);
int (*get_fsinfo) (struct dentry *, struct fsinfo_kparams *);
int (*remount_fs) (struct super_block *, int *, char *, size_t);
- int (*reconfigure) (struct super_block *, struct fs_context *);
void (*umount_begin) (struct super_block *);
int (*show_options)(struct seq_file *, struct dentry *);
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 9a6aa6bcf745..5e79c33ade7d 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -34,6 +34,8 @@ enum fs_context_purpose {
FS_CONTEXT_FOR_KERNEL_MOUNT, /* New superblock for kernel-internal mount */
FS_CONTEXT_FOR_SUBMOUNT, /* New superblock for automatic submount */
FS_CONTEXT_FOR_RECONFIGURE, /* Superblock reconfiguration (remount) */
+ FS_CONTEXT_FOR_UMOUNT, /* Reconfiguration to R/O for unmount */
+ FS_CONTEXT_FOR_EMERGENCY_RO, /* Emergency reconfiguration to R/O */
};
/*
@@ -102,6 +104,7 @@ struct fs_context {
void *security; /* The LSM context */
void *s_fs_info; /* Proposed s_fs_info */
unsigned int sb_flags; /* Proposed superblock flags (SB_*) */
+ unsigned int sb_flags_mask; /* Superblock flags that were changed */
enum fs_context_purpose purpose:8;
enum fs_context_phase phase:8; /* The phase the context is in */
bool sloppy:1; /* T if unrecognised options are okay */
@@ -116,6 +119,7 @@ struct fs_context_operations {
int (*parse_monolithic)(struct fs_context *fc, void *data, size_t data_size);
int (*validate)(struct fs_context *fc);
int (*get_tree)(struct fs_context *fc);
+ int (*reconfigure)(struct fs_context *fc);
};
/*
@@ -123,9 +127,9 @@ struct fs_context_operations {
*/
extern struct fs_context *vfs_new_fs_context(struct file_system_type *fs_type,
struct dentry *reference,
- unsigned int ms_flags,
+ unsigned int sb_flags,
+ unsigned int sb_flags_mask,
enum fs_context_purpose purpose);
-extern struct fs_context *vfs_sb_reconfig(struct path *path, unsigned int ms_flags);
extern struct fs_context *vfs_dup_fs_context(struct fs_context *src);
extern int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param);
extern int vfs_parse_fs_string(struct fs_context *fc, const char *key,
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 9fdcdbbb67e9..a6da692792a3 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -370,6 +370,7 @@ int kernfs_get_tree(struct fs_context *fc);
void kernfs_free_fs_context(struct fs_context *fc);
void kernfs_kill_sb(struct super_block *sb);
struct super_block *kernfs_pin_sb(struct kernfs_root *root, const void *ns);
+int kernfs_reconfigure(struct fs_context *fc);
void kernfs_init(void);
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index b1a62dc0b8d9..3cfa89f41bad 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -160,13 +160,6 @@
* @orig_data is the size of the original data
* @copy copied data which will be passed to the security module.
* Returns 0 if the copy was successful.
- * @sb_remount:
- * Extracts security system specific mount options and verifies no changes
- * are being made to those options.
- * @sb superblock being remounted
- * @data contains the filesystem-specific data.
- * @data_size contains the size of the data.
- * Return 0 if permission is granted.
* @sb_umount:
* Check permission before the @mnt file system is unmounted.
* @mnt contains the mounted file system.
@@ -1518,7 +1511,6 @@ union security_list_options {
int (*sb_alloc_security)(struct super_block *sb);
void (*sb_free_security)(struct super_block *sb);
int (*sb_copy_data)(char *orig, size_t orig_size, char *copy);
- int (*sb_remount)(struct super_block *sb, void *data, size_t data_size);
int (*sb_show_options)(struct seq_file *m, struct super_block *sb);
int (*sb_statfs)(struct dentry *dentry);
int (*sb_mount)(const char *dev_name, const struct path *path,
@@ -1865,7 +1857,6 @@ struct security_hook_heads {
struct hlist_head sb_alloc_security;
struct hlist_head sb_free_security;
struct hlist_head sb_copy_data;
- struct hlist_head sb_remount;
struct hlist_head sb_show_options;
struct hlist_head sb_statfs;
struct hlist_head sb_mount;
diff --git a/include/linux/security.h b/include/linux/security.h
index c73d9ba863bc..047b2cff1209 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -240,7 +240,6 @@ int security_sb_mountpoint(struct fs_context *fc, struct path *mountpoint,
int security_sb_alloc(struct super_block *sb);
void security_sb_free(struct super_block *sb);
int security_sb_copy_data(char *orig, size_t orig_size, char *copy);
-int security_sb_remount(struct super_block *sb, void *data, size_t data_size);
int security_sb_show_options(struct seq_file *m, struct super_block *sb);
int security_sb_statfs(struct dentry *dentry);
int security_sb_mount(const char *dev_name, const struct path *path,
@@ -585,11 +584,6 @@ static inline int security_sb_copy_data(char *orig, size_t orig_size, char *copy
return 0;
}
-static inline int security_sb_remount(struct super_block *sb, void *data, size_t data_size)
-{
- return 0;
-}
-
static inline int security_sb_show_options(struct seq_file *m,
struct super_block *sb)
{
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 0f102210f89e..0ac430f48800 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -403,7 +403,7 @@ static struct vfsmount *mq_create_mount(struct ipc_namespace *ns)
struct vfsmount *mnt;
int ret;
- fc = vfs_new_fs_context(&mqueue_fs_type, NULL, 0,
+ fc = vfs_new_fs_context(&mqueue_fs_type, NULL, 0, 0,
FS_CONTEXT_FOR_KERNEL_MOUNT);
if (IS_ERR(fc))
return ERR_CAST(fc);
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 958b3fd81c56..6542c0c3e32f 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2150,6 +2150,7 @@ static const struct fs_context_operations cgroup_fs_context_ops = {
.parse_param = cgroup_parse_param,
.validate = cgroup_validate,
.get_tree = cgroup_get_tree,
+ .reconfigure = kernfs_reconfigure,
};
/*
@@ -5281,7 +5282,6 @@ int cgroup_rmdir(struct kernfs_node *kn)
static struct kernfs_syscall_ops cgroup_kf_syscall_ops = {
.show_options = cgroup_show_options,
.fsinfo = cgroup_fsinfo,
- .reconfigure = cgroup_reconfigure,
.mkdir = cgroup_mkdir,
.rmdir = cgroup_rmdir,
.show_path = cgroup_show_path,
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index b02161a41d5a..b4ad1a52f006 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -327,7 +327,8 @@ static int cpuset_get_tree(struct fs_context *fc)
if (!cgroup_fs)
goto out;
- cg_fc = vfs_new_fs_context(cgroup_fs, NULL, fc->sb_flags, fc->purpose);
+ cg_fc = vfs_new_fs_context(cgroup_fs, NULL, fc->sb_flags, fc->sb_flags,
+ fc->purpose);
put_filesystem(cgroup_fs);
if (IS_ERR(cg_fc)) {
ret = PTR_ERR(cg_fc);
diff --git a/security/security.c b/security/security.c
index 2439a5613813..95b348484c5a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -415,11 +415,6 @@ int security_sb_copy_data(char *orig, size_t data_size, char *copy)
}
EXPORT_SYMBOL(security_sb_copy_data);
-int security_sb_remount(struct super_block *sb, void *data, size_t data_size)
-{
- return call_int_hook(sb_remount, 0, sb, data, data_size);
-}
-
int security_sb_show_options(struct seq_file *m, struct super_block *sb)
{
return call_int_hook(sb_show_options, 0, m, sb);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3d5b09c256c1..d9cfb8b2fca4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7180,7 +7180,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
LSM_HOOK_INIT(sb_copy_data, selinux_sb_copy_data),
- LSM_HOOK_INIT(sb_remount, selinux_sb_remount),
LSM_HOOK_INIT(sb_show_options, selinux_sb_show_options),
LSM_HOOK_INIT(sb_statfs, selinux_sb_statfs),
LSM_HOOK_INIT(sb_mount, selinux_mount),
^ permalink raw reply related
* Re: [PATCH 28/33] vfs: syscall: Add fsconfig() for configuring and managing a context [ver #11]
From: David Howells @ 2018-08-09 14:24 UTC (permalink / raw)
To: Eric W. Biederman
Cc: dhowells, viro, linux-api, torvalds, linux-fsdevel, linux-kernel
In-Reply-To: <87in4n9zg0.fsf@xmission.com>
Eric W. Biederman <ebiederm@xmission.com> wrote:
> First let me thank you for adding both FSCONFIG_CMD_CREATE and
> FSCONFIG_CMD_RECONFIGURE. Unfortunately the implementation is currently
> broken. So this patch gets my:
>
> This is broken in two specific ways.
> ...
> 2) FSCONFIG_CMD_CREATE will succeed even if the superblock already
> exists and it can not use all of the superblock parameters.
>
> This happens because vfs_get_super will only call fill_super
> if the super block is created. Which is reasonable on the face
> of it. But it in practice this introduces security problems.
>
> a) Either through reconfiguring a shared super block you did not
> realize was shared (as we saw with devpts).
>
> b) Mounting a super block and not honoring it's mount options
> because something has already mounted it. As we see today
> with proc. Leaving userspace to think the filesystem will behave
> one way when in fact it behaves another.
>
> I have already explained this several times, and apparently I have been
> ignored. This fundamental usability issue that leads to security
> problems.
I've also explained why you're wrong or at least only partially right. I *do*
*not* want to implement sget() in userspace with the ability for userspace to
lock out other mount requests - which is what it appears that you've been
asking for.
However, as I have said, I *am* willing to add one of more flags to help with
this, but I can't make any "legacy" fs honour them as this requires the
fs_context to be passed down to sget_fc() and the filesystem - which is why I
was considering leaving it for later.
(1) An FSOPEN_EXCL flag to tell sget_fc() to fail if the superblock already
exists at all.
(2) An FSOPEN_FAIL_ON_MISMATCH flag to explicitly say that we *don't* want a
superblock with different parameters.
The implication of providing neither flag is that we are happy to accept a
superblock from the same source but with different parameters.
But it doesn't seem to be an absolute imperative to roll this out immediately,
since what I have exactly mirrors what the kernel currently does - and forcing
a change in that behaviour risks breaking userspace. If it keeps you happy,
however, I can try and work one up.
David
^ permalink raw reply
* Re: [PATCH 28/33] vfs: syscall: Add fsconfig() for configuring and managing a context [ver #11]
From: Miklos Szeredi @ 2018-08-09 14:35 UTC (permalink / raw)
To: David Howells
Cc: Eric W. Biederman, Al Viro, Linux API, Linus Torvalds,
linux-fsdevel, linux-kernel
In-Reply-To: <27374.1533824694@warthog.procyon.org.uk>
On Thu, Aug 9, 2018 at 4:24 PM, David Howells <dhowells@redhat.com> wrote:
> Eric W. Biederman <ebiederm@xmission.com> wrote:
>
>> First let me thank you for adding both FSCONFIG_CMD_CREATE and
>> FSCONFIG_CMD_RECONFIGURE. Unfortunately the implementation is currently
>> broken. So this patch gets my:
>>
>> This is broken in two specific ways.
>> ...
>> 2) FSCONFIG_CMD_CREATE will succeed even if the superblock already
>> exists and it can not use all of the superblock parameters.
>>
>> This happens because vfs_get_super will only call fill_super
>> if the super block is created. Which is reasonable on the face
>> of it. But it in practice this introduces security problems.
>>
>> a) Either through reconfiguring a shared super block you did not
>> realize was shared (as we saw with devpts).
>>
>> b) Mounting a super block and not honoring it's mount options
>> because something has already mounted it. As we see today
>> with proc. Leaving userspace to think the filesystem will behave
>> one way when in fact it behaves another.
>>
>> I have already explained this several times, and apparently I have been
>> ignored. This fundamental usability issue that leads to security
>> problems.
>
> I've also explained why you're wrong or at least only partially right. I *do*
> *not* want to implement sget() in userspace with the ability for userspace to
> lock out other mount requests - which is what it appears that you've been
> asking for.
>
> However, as I have said, I *am* willing to add one of more flags to help with
> this, but I can't make any "legacy" fs honour them as this requires the
> fs_context to be passed down to sget_fc() and the filesystem - which is why I
> was considering leaving it for later.
You can determine at fsopen() time whether the filesystem is able to
support the O_EXCL behavior? If so, then it's trivial to enable this
conditionally. I think that's what Eric is asking for, it's obviously
not fair to ask for a change in behavior of the legacy interface.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH 28/33] vfs: syscall: Add fsconfig() for configuring and managing a context [ver #11]
From: Eric W. Biederman @ 2018-08-09 15:32 UTC (permalink / raw)
To: David Howells; +Cc: viro, linux-api, torvalds, linux-fsdevel, linux-kernel
In-Reply-To: <27374.1533824694@warthog.procyon.org.uk>
David Howells <dhowells@redhat.com> writes:
> Eric W. Biederman <ebiederm@xmission.com> wrote:
>
>> First let me thank you for adding both FSCONFIG_CMD_CREATE and
>> FSCONFIG_CMD_RECONFIGURE. Unfortunately the implementation is currently
>> broken. So this patch gets my:
>>
>> This is broken in two specific ways.
>> ...
>> 2) FSCONFIG_CMD_CREATE will succeed even if the superblock already
>> exists and it can not use all of the superblock parameters.
>>
>> This happens because vfs_get_super will only call fill_super
>> if the super block is created. Which is reasonable on the face
>> of it. But it in practice this introduces security problems.
>>
>> a) Either through reconfiguring a shared super block you did not
>> realize was shared (as we saw with devpts).
>>
>> b) Mounting a super block and not honoring it's mount options
>> because something has already mounted it. As we see today
>> with proc. Leaving userspace to think the filesystem will behave
>> one way when in fact it behaves another.
>>
>> I have already explained this several times, and apparently I have been
>> ignored. This fundamental usability issue that leads to security
>> problems.
>
> I've also explained why you're wrong or at least only partially right. I *do*
> *not* want to implement sget() in userspace with the ability for userspace to
> lock out other mount requests - which is what it appears that you've been
> asking for.
All I really care about is that when you ask for a set of paramaters
that you get a filesystem with that set of parameters. Not the same
filsystem mounted a different way.
That has gone wrong twice badly. There is no common case I know of that
requires returning the same filesystem twice. AKA the pain of the
existing semantics seems much much worse than any benefit. So I am
asking that we not propagate the existing semantics into the new API.
You are cleaning up dealing with mount options and this is one of the
places where they need cleaning up.
> However, as I have said, I *am* willing to add one of more flags to help with
> this, but I can't make any "legacy" fs honour them as this requires the
> fs_context to be passed down to sget_fc() and the filesystem - which is why I
> was considering leaving it for later.
>
> (1) An FSOPEN_EXCL flag to tell sget_fc() to fail if the superblock already
> exists at all.
>
> (2) An FSOPEN_FAIL_ON_MISMATCH flag to explicitly say that we *don't* want a
> superblock with different parameters.
>
> The implication of providing neither flag is that we are happy to accept a
> superblock from the same source but with different parameters.
>
> But it doesn't seem to be an absolute imperative to roll this out immediately,
> since what I have exactly mirrors what the kernel currently does - and forcing
> a change in that behaviour risks breaking userspace. If it keeps you happy,
> however, I can try and work one up.
What I am asking is that the default behavior for the new API when using
FSCONFIG_CMD_CREATE is to call sget_fc with either FSOPEN_EXCL or
FSOPEN_FAIL_ON_MISMATCH. I know FSOPEN_EXCL is trivial to implement. I
don't know if there are any hidden gotcha's with
FSOPEN_FAIL_ON_MISMATCH.
This change in default behavior for your patch needs to be implemented
before this hits a released kernel. Returning a filesystem with
different than the requested parameters has resulted in at least two
major issues, that are very hard to clean up after the fact. A chroot
system changing the parameters on /dev/pts resulting in some
distributions keeping the suid pt_chown binary long past it's best buy
date, and other distributions instead choosing to break userspace. Then
there is the current issue where in practice proc does not any of it's
mount paramaters which breaks the android security model.
The fact that these things happen silently and you have to be on your
toes to catch them is fundamentally a bug in the API. If the mount
request had simply failed people would have noticed the issues much
sooner and silently b0rkend configuration would not have propagated. As
such I do not believe we should propagate this misfeature from the old
API into the new API.
Conceptually I like FSOPEN_FAIL_ON_MISMATH as it looks like it is
sufficient to the needs, and with a little luck we could even change
the old API to those semantics.
Ultimately I want to close a giant mental model mismatch.
User: I am creating the data structures to read filesystem X
with parameters Y.
Kernel: He wants filesystem X. If it is a slow day use parameters Y.
Given that historically the reuse of a superblock did not exist, and
that in practice it almost never happens. It is quite reqsonable for
users to not expect the kernel to completely ignore the mount parameters
they pass to the kernel.
So please let's fix that now when it is easy.
Eric
^ permalink raw reply
* Re: [PATCH 28/33] vfs: syscall: Add fsconfig() for configuring and managing a context [ver #11]
From: David Howells @ 2018-08-09 16:33 UTC (permalink / raw)
To: Miklos Szeredi
Cc: dhowells, Eric W. Biederman, Al Viro, Linux API, Linus Torvalds,
linux-fsdevel, linux-kernel
In-Reply-To: <CAJfpegvWE9htLjqeR6=2BWBSuvJzJpWcjBC_EmX_k1RCGXTfbw@mail.gmail.com>
Miklos Szeredi <miklos@szeredi.hu> wrote:
> > However, as I have said, I *am* willing to add one of more flags to help
> > with this, but I can't make any "legacy" fs honour them as this requires
> > the fs_context to be passed down to sget_fc() and the filesystem - which
> > is why I was considering leaving it for later.
>
> You can determine at fsopen() time whether the filesystem is able to
> support the O_EXCL behavior? If so, then it's trivial to enable this
> conditionally. I think that's what Eric is asking for, it's obviously
> not fair to ask for a change in behavior of the legacy interface.
What do you mean by "enable it conditionally"? It cannot be enabled for
filesystems that don't pass fs_context down to sget().
mount(2) mustn't enable it lest it break userspace.
fsopen(2) can let userspace set a flag to enable it.
David
^ permalink raw reply
* Re: [PATCH v4 0/4] seccomp trap to userspace
From: Dinesh Subhraveti @ 2018-08-10 0:31 UTC (permalink / raw)
To: linux-api, linux-kernel
In-Reply-To: <20180807024442.GA12274@cisco.lan>
On Mon, Aug 6, 2018 at 7:44 PM Tycho Andersen <tycho@tycho.ws> wrote:
>
> Hi all,
>
> Dinesh Subhraveti has claimed that some part of this series might be
> patented. While he has not furnished me with anything to confirm this
> claim, I'll put this series on hold.
For the sake of everyone's clarity, there is no patent or intellectual
property issue here and we'd like to see this feature in the kernel.
I did indicate that there is a patent related to the mechanism
underlying this patch set but that is completely incidental to the
issue here. We have filed that patent only for the purpose of defense
and have no interest or resources to go after infringements. As such,
I spoke about the approach, it's value for networking and a few
possible ways of implementing it in the kernel at Linux Plumbers
Conference 2017.
As a contractor and a member of our team at AppSwitch (FKA Fermat),
Tycho Andersen helped implement a fully user space version of system
call trap mechanism based on seccomp / fd-passing and participated in
our team discussions about upstreaming a kernel version of the
feature. Given that context, we were taken aback that he posted the
v1 patch set without letting us know and without any mention of
AppSwitch in the post even though he was still under contract at that
time.
In any case, please note that we will communicate with Tycho directly
regarding this matter going forward.
--
Dinesh Subhraveti
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox