* Re: [PATCH] fs, proc: introduce CONFIG_PROC_CHILDREN
From: Djalal Harouni @ 2015-05-26 15:07 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: Iago López Galeiras, Oleg Nesterov, Kees Cook,
Pavel Emelyanov, Serge Hallyn, KAMEZAWA Hiroyuki, linux-api,
Alexander Viro, linux-fsdevel, Andrew Morton, Andy Lutomirski,
Alban Crequy
In-Reply-To: <20150522153705.GI16716@uranus>
On Fri, May 22, 2015 at 06:37:05PM +0300, Cyrill Gorcunov wrote:
> On Fri, May 22, 2015 at 02:47:39PM +0200, Iago López Galeiras wrote:
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1145,6 +1145,7 @@ endif # CGROUPS
> >
> > config CHECKPOINT_RESTORE
> > bool "Checkpoint/restore support" if EXPERT
> > + depends on PROC_CHILDREN
> > default n
> > help
> > Enables additional kernel features in a sake of checkpoint/restore.
>
> Maybe better select PROC_CHILDREN here? Also I must admit I don't understand
> why similar patch from Andy has not been merged earlier? Andy, there was
> some problem I forgot about?
From the linked thread, it seems that it was just lost... The discussion
ended up about task/<tid> and /proc/<pid>... That patch should have been
applied.
BTW this patch adds the CONFIG guard that was requested by Andrew. There
is a v3 coming to make it "select" as suggested.
Thanks!
--
Djalal Harouni
http://opendz.org
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3] fs, proc: introduce CONFIG_PROC_CHILDREN
From: Iago López Galeiras @ 2015-05-26 15:12 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: Iago López Galeiras, Oleg Nesterov, Kees Cook,
Pavel Emelyanov, Serge Hallyn, KAMEZAWA Hiroyuki, linux-api,
Alexander Viro, linux-fsdevel, Andrew Morton, Andy Lutomirski,
Djalal Harouni, Alban Crequy
In-Reply-To: <20150522153705.GI16716@uranus>
commit 818411616baf ("fs, proc: introduce
/proc/<pid>/task/<tid>/children entry") introduced the children entry
for checkpoint restore and the file is only available on kernels
configured with CONFIG_EXPERT and CONFIG_CHECKPOINT_RESTORE.
This is available in most distributions (Fedora, Debian, Ubuntu, CoreOS)
because they usually enable CONFIG_EXPERT and CONFIG_CHECKPOINT_RESTORE.
But Arch does not enable CONFIG_EXPERT or CONFIG_CHECKPOINT_RESTORE.
However, the children proc file is useful outside of checkpoint restore.
I would like to use it in rkt. The rkt process exec() another program it
does not control, and that other program will fork()+exec() a child
process. I would like to find the pid of the child process from an
external tool without iterating in /proc over all processes to find
which one has a parent pid equal to rkt.
This commit introduces CONFIG_PROC_CHILDREN and makes
CONFIG_CHECKPOINT_RESTORE select it. This allows enabling
/proc/<pid>/task/<tid>/children without needing to enable
CONFIG_CHECKPOINT_RESTORE and CONFIG_EXPERT.
Alban tested that /proc/<pid>/task/<tid>/children is present when the
kernel is configured with CONFIG_PROC_CHILDREN=y but without
CONFIG_CHECKPOINT_RESTORE
v3: change depend -> select
v2: introduce CONFIG_PROC_CHILDREN http://marc.info/?l=linux-fsdevel&m=143229890217280&w=2
v1: http://marc.info/?l=linux-api&m=143220431121869&w=2
Signed-off-by: Iago López Galeiras <iago@endocode.com>
Cc: Alban Crequy <alban@endocode.com>
Tested-by: Alban Crequy <alban@endocode.com>
---
fs/proc/Kconfig | 4 ++++
fs/proc/array.c | 4 ++--
fs/proc/base.c | 2 +-
init/Kconfig | 1 +
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig
index 2183fcf..d751fcb 100644
--- a/fs/proc/Kconfig
+++ b/fs/proc/Kconfig
@@ -71,3 +71,7 @@ config PROC_PAGE_MONITOR
/proc/pid/smaps, /proc/pid/clear_refs, /proc/pid/pagemap,
/proc/kpagecount, and /proc/kpageflags. Disabling these
interfaces will reduce the size of the kernel by approximately 4kb.
+
+config PROC_CHILDREN
+ bool "Include /proc/<pid>/task/<tid>/children file"
+ default n
diff --git a/fs/proc/array.c b/fs/proc/array.c
index fd02a9e..ddc4b2c 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -569,7 +569,7 @@ int proc_pid_statm(struct seq_file *m, struct pid_namespace *ns,
return 0;
}
-#ifdef CONFIG_CHECKPOINT_RESTORE
+#ifdef CONFIG_PROC_CHILDREN
static struct pid *
get_children_pid(struct inode *inode, struct pid *pid_prev, loff_t pos)
{
@@ -692,4 +692,4 @@ const struct file_operations proc_tid_children_operations = {
.llseek = seq_lseek,
.release = children_seq_release,
};
-#endif /* CONFIG_CHECKPOINT_RESTORE */
+#endif /* CONFIG_PROC_CHILDREN */
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 093ca14..5477a4a 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2922,7 +2922,7 @@ static const struct pid_entry tid_base_stuff[] = {
ONE("stat", S_IRUGO, proc_tid_stat),
ONE("statm", S_IRUGO, proc_pid_statm),
REG("maps", S_IRUGO, proc_tid_maps_operations),
-#ifdef CONFIG_CHECKPOINT_RESTORE
+#ifdef CONFIG_PROC_CHILDREN
REG("children", S_IRUGO, proc_tid_children_operations),
#endif
#ifdef CONFIG_NUMA
diff --git a/init/Kconfig b/init/Kconfig
index dc24dec..fa09e5e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1145,6 +1145,7 @@ endif # CGROUPS
config CHECKPOINT_RESTORE
bool "Checkpoint/restore support" if EXPERT
+ select PROC_CHILDREN
default n
help
Enables additional kernel features in a sake of checkpoint/restore.
--
2.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2 1/2] capabilities: Ambient capabilities
From: Christoph Lameter @ 2015-05-26 16:23 UTC (permalink / raw)
To: Serge Hallyn
Cc: Andy Lutomirski, Serge Hallyn, Andrew Morton, Jarkko Sakkinen,
Ted Ts'o, Andrew G. Morgan, Linux API, Mimi Zohar,
Michael Kerrisk, Austin S Hemmelgarn, linux-security-module,
Aaron Jones, Serge Hallyn, LKML, Markku Savela, Kees Cook,
Jonathan Corbet, Andy Lutomirski
In-Reply-To: <20150523193705.GA30563-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
On Sat, 23 May 2015, Serge Hallyn wrote:
> > You cannot use pA to try to subvert a setuid, setgid, or file-capped
> > program: if you execute any such program, pA gets cleared and the
> > resulting evolution rules are unchanged by this patch.
>
> Christoph, just to be sure, is this ^ going to suffice for you?
>
> Seems like it should since any program which is setuid-root, i.e.
> passwd, isn't likely to be designed to exec other programs.
Yes that should work.
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Lukasz Pawelczyk @ 2015-05-26 16:27 UTC (permalink / raw)
To: Stephen Smalley
Cc: Lukasz Pawelczyk, linux-doc-u79uwXL29TY76Z2rM5mHXA, Tetsuo Handa,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Paul Moore,
Jonathan Corbet, Jingoo Han, Jeff Layton, Alexey Dobriyan,
Kees Cook, Mauro Carvalho Chehab, Fabian Frederick,
Casey Schaufler, Al Viro, James Morris, Kirill A. Shutemov,
John Johansen, Rafal Krypa, Greg KH,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Miklos Szeredi, Oleg Nesterov, Andy
In-Reply-To: <556484BD.2060004-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>
Hi,
Thanks for taking the interest and commenting on this.
Replies below.
On wto, 2015-05-26 at 10:35 -0400, Stephen Smalley wrote:
> On 05/25/2015 08:32 AM, Lukasz Pawelczyk wrote:
> > --- Design ideas ---
> >
> > "Smack namespace" is rather "Smack labels namespace" as not the whole
> > MAC is namespaced, only the labels. There is a great analogy between
> > Smack labels namespace and the user namespace part that remaps UIDs.
> >
> > The idea is to create a map of labels for a namespace so the namespace
> > is only allowed to use those labels. Smack rules are always the same
> > as in the init namespace (limited only by what labels are mapped) and
> > cannot be manipulated from the child namespace. The map is actually
> > only for labels' names. The underlying structures for labels remain
> > the same. The filesystem also stores the "unmapped" labels from the
> > init namespace.
>
> How do you achieve that without introducing additional hooks or
> reworking the current hooks in the setxattr code path? At present, the
> security module is allowed to rewrite getxattr requests on the
> security.* namespace but it isn't allowed to do that for setxattr, so if
> the process invokes setxattr with a mapped label, then it will be the
> mapped label that gets passed to the filesystem implementation, not the
> unmapped label. The security module may internally store it in unmapped
> form and may even return that upon getxattr() calls, but if you then
> reboot the system and later fetch from the filesystem, it will get the
> mapped label value.
I call the inode operation by hand in the post_setxattr.
The label will effectively be set twice, which is not ideal, but there
is no other option right now without reworking the hooks as you said.
This shouldn't really be a problem because the Smack operations will not
use the filesystem label (even when it's set incorrectly for a moment)
but an already initialized smack_known structure for this inode that has
all the values filled in properly.
The only attack vector I can think of is hard rebooting the machine in a
way that mapped label is really saved in the filesystem before the
unmapped will have a chance. Should I be worried about that? This sounds
a little unreal.
@@ -1163,12 +1214,24 @@ static void smack_inode_post_setxattr(struct dentry *dentry, const char *name,
skpp = &isp->smk_mmap;
if (skpp) {
- skp = smk_get_label(value, size, true);
+ skp = smk_get_label(value, size, true, ns);
if (!IS_ERR(skp))
*skpp = skp;
else
*skpp = &smack_known_invalid;
+
+ /*
+ * The label we get above might be a different than the one
+ * kernel has already set before calling this function.
+ * Be consistent and set the final value in the filesystem.
+ * The cases for this are errors and labels being used
+ * in a namespace where we want to store an unmapped
+ * value in the filesystem.
+ */
+ dentry->d_inode->i_op->setxattr(dentry, name,
+ (*skpp)->smk_known,
+ size, flags);
}
}
>
> > --- Usage ---
> >
> > Smack namespace is written using LSM hooks inside user namespace. That
> > means it's connected to it.
> >
> > To create a new Smack namespace you need to unshare() user namespace
> > as usual. If that is all you do though, than there is no difference to
> > what is now. To activate the Smack namespace you need to fill the
> > labels' map. It is in a file /proc/$PID/smack_map.
>
> This should be /proc/$PID/attr/label_map or similar, modeled after the
> existing /proc/$PID/attr/current and similar nodes. Then it isn't
> module-specific and can be reused for other modules.
Sure. I had some thoughts about that, but couldn't really figure out
which option would be best. I'll change it as suggested.
> > Writing to the map file is not disabled after the first write as it is
> > in uid_map. For Smack we have no means to map ranges of labels, hence
> > it can really be advantageous to be able to expand the map later
> > on. But you can only add to the map. You cannot remove already mapped
> > labels. You cannot change the already existing mappings. Also mappings
> > has to be 1-1. All requests to create a map where either the unmapped
> > or the mapped label already exists in the map will be denied.
>
> Isn't it a concern that I can then add additional labels to the mapping
> for which I am not authorized? Or is this mitigated by the fact that I
> cannot alter the rules?
You can't add mappings for which you are not authorized.
This is mitigated by both, by the fact that adding a mapping is a
privileged operation in the parent (only init for now) namespace and
that modifying the rules is a privileged operation allowed only in the
init namespace (as it is now).
> What about the situation for the predefined
> labels in Smack - are you assuming that they will always be mapped up
> front in the mapping file?
If somebody wants to have a full Smack aware container with full Smack
functionality that's what I'd expect, but it's not mandatory.
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Stephen Smalley @ 2015-05-26 16:34 UTC (permalink / raw)
To: Lukasz Pawelczyk
Cc: David S. Miller, Eric W. Biederman, Kirill A. Shutemov,
Serge E. Hallyn, Al Viro, Alexey Dobriyan, Andrew Morton,
Andy Lutomirski, Casey Schaufler, David Howells, Fabian Frederick,
Greg KH, James Morris, Jeff Layton, Jingoo Han, Joe Perches,
John Johansen, Jonathan Corbet, Kees Cook, Mauro Carvalho Chehab,
Miklos Szeredi, Oleg Nesterov, Paul Moore
In-Reply-To: <1432657655.1974.21.camel@samsung.com>
On 05/26/2015 12:27 PM, Lukasz Pawelczyk wrote:
> Hi,
>
> Thanks for taking the interest and commenting on this.
> Replies below.
>
>
> On wto, 2015-05-26 at 10:35 -0400, Stephen Smalley wrote:
>> On 05/25/2015 08:32 AM, Lukasz Pawelczyk wrote:
>>> --- Design ideas ---
>>>
>>> "Smack namespace" is rather "Smack labels namespace" as not the whole
>>> MAC is namespaced, only the labels. There is a great analogy between
>>> Smack labels namespace and the user namespace part that remaps UIDs.
>>>
>>> The idea is to create a map of labels for a namespace so the namespace
>>> is only allowed to use those labels. Smack rules are always the same
>>> as in the init namespace (limited only by what labels are mapped) and
>>> cannot be manipulated from the child namespace. The map is actually
>>> only for labels' names. The underlying structures for labels remain
>>> the same. The filesystem also stores the "unmapped" labels from the
>>> init namespace.
>>
>> How do you achieve that without introducing additional hooks or
>> reworking the current hooks in the setxattr code path? At present, the
>> security module is allowed to rewrite getxattr requests on the
>> security.* namespace but it isn't allowed to do that for setxattr, so if
>> the process invokes setxattr with a mapped label, then it will be the
>> mapped label that gets passed to the filesystem implementation, not the
>> unmapped label. The security module may internally store it in unmapped
>> form and may even return that upon getxattr() calls, but if you then
>> reboot the system and later fetch from the filesystem, it will get the
>> mapped label value.
>
> I call the inode operation by hand in the post_setxattr.
>
> The label will effectively be set twice, which is not ideal, but there
> is no other option right now without reworking the hooks as you said.
>
> This shouldn't really be a problem because the Smack operations will not
> use the filesystem label (even when it's set incorrectly for a moment)
> but an already initialized smack_known structure for this inode that has
> all the values filled in properly.
>
> The only attack vector I can think of is hard rebooting the machine in a
> way that mapped label is really saved in the filesystem before the
> unmapped will have a chance. Should I be worried about that? This sounds
> a little unreal.
If it were my security module, I would be worried about it. Even aside
from maliciously induced failure, you are leaving yourself open to
inconsistencies arising upon crashes. I would suggest modifying the
setxattr hook so that the security module can override the original
value/size pair with its own definition before it is passed to the inode
operation. There is already precedent in that security modules are
allowed to override the value/size returned by getxattr for security.*,
so this just makes them fully parallel.
^ permalink raw reply
* Re: [RFC v2 1/4] fs: Add generic file system event notifications
From: Beata Michalska @ 2015-05-26 16:39 UTC (permalink / raw)
To: Greg KH
Cc: Jan Kara, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, tytso-3s7WtUTddSA,
adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q,
hughd-hpIqsD4AKlfQT0dZR+AlfA, lczerner-H+wXaHxf7aLQT0dZR+AlfA,
hch-wEGCiKHe2LqWVfeAwA7xHQ, linux-ext4-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
kmpark-wEGCiKHe2LqWVfeAwA7xHQ
In-Reply-To: <554B5329.8040907-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Hi,
On 05/07/2015 01:57 PM, Beata Michalska wrote:
> Hi,
>
> On 05/05/2015 02:16 PM, Beata Michalska wrote:
>> Hi again,
>>
>> On 04/29/2015 11:13 AM, Greg KH wrote:
>>> On Wed, Apr 29, 2015 at 09:42:59AM +0200, Jan Kara wrote:
>>>> On Wed 29-04-15 09:03:08, Beata Michalska wrote:
>>>>> On 04/28/2015 07:39 PM, Greg KH wrote:
>>>>>> On Tue, Apr 28, 2015 at 04:46:46PM +0200, Beata Michalska wrote:
>>>>>>> On 04/28/2015 04:09 PM, Greg KH wrote:
>>>>>>>> On Tue, Apr 28, 2015 at 03:56:53PM +0200, Jan Kara wrote:
>>>>>>>>> On Mon 27-04-15 17:37:11, Greg KH wrote:
>>>>>>>>>> On Mon, Apr 27, 2015 at 05:08:27PM +0200, Beata Michalska wrote:
>>>>>>>>>>> On 04/27/2015 04:24 PM, Greg KH wrote:
>>>>>>>>>>>> On Mon, Apr 27, 2015 at 01:51:41PM +0200, Beata Michalska wrote:
>>>>>>>>>>>>> Introduce configurable generic interface for file
>>>>>>>>>>>>> system-wide event notifications, to provide file
>>>>>>>>>>>>> systems with a common way of reporting any potential
>>>>>>>>>>>>> issues as they emerge.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The notifications are to be issued through generic
>>>>>>>>>>>>> netlink interface by newly introduced multicast group.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Threshold notifications have been included, allowing
>>>>>>>>>>>>> triggering an event whenever the amount of free space drops
>>>>>>>>>>>>> below a certain level - or levels to be more precise as two
>>>>>>>>>>>>> of them are being supported: the lower and the upper range.
>>>>>>>>>>>>> The notifications work both ways: once the threshold level
>>>>>>>>>>>>> has been reached, an event shall be generated whenever
>>>>>>>>>>>>> the number of available blocks goes up again re-activating
>>>>>>>>>>>>> the threshold.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The interface has been exposed through a vfs. Once mounted,
>>>>>>>>>>>>> it serves as an entry point for the set-up where one can
>>>>>>>>>>>>> register for particular file system events.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: Beata Michalska <b.michalska-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>> Documentation/filesystems/events.txt | 231 ++++++++++
>>>>>>>>>>>>> fs/Makefile | 1 +
>>>>>>>>>>>>> fs/events/Makefile | 6 +
>>>>>>>>>>>>> fs/events/fs_event.c | 770 ++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>> fs/events/fs_event.h | 25 ++
>>>>>>>>>>>>> fs/events/fs_event_netlink.c | 99 +++++
>>>>>>>>>>>>> fs/namespace.c | 1 +
>>>>>>>>>>>>> include/linux/fs.h | 6 +-
>>>>>>>>>>>>> include/linux/fs_event.h | 58 +++
>>>>>>>>>>>>> include/uapi/linux/fs_event.h | 54 +++
>>>>>>>>>>>>> include/uapi/linux/genetlink.h | 1 +
>>>>>>>>>>>>> net/netlink/genetlink.c | 7 +-
>>>>>>>>>>>>> 12 files changed, 1257 insertions(+), 2 deletions(-)
>>>>>>>>>>>>> create mode 100644 Documentation/filesystems/events.txt
>>>>>>>>>>>>> create mode 100644 fs/events/Makefile
>>>>>>>>>>>>> create mode 100644 fs/events/fs_event.c
>>>>>>>>>>>>> create mode 100644 fs/events/fs_event.h
>>>>>>>>>>>>> create mode 100644 fs/events/fs_event_netlink.c
>>>>>>>>>>>>> create mode 100644 include/linux/fs_event.h
>>>>>>>>>>>>> create mode 100644 include/uapi/linux/fs_event.h
>>>>>>>>>>>>
>>>>>>>>>>>> Any reason why you just don't do uevents for the block devices today,
>>>>>>>>>>>> and not create a new type of netlink message and userspace tool required
>>>>>>>>>>>> to read these?
>>>>>>>>>>>
>>>>>>>>>>> The idea here is to have support for filesystems with no backing device as well.
>>>>>>>>>>> Parsing the message with libnl is really simple and requires few lines of code
>>>>>>>>>>> (sample application has been presented in the initial version of this RFC)
>>>>>>>>>>
>>>>>>>>>> I'm not saying it's not "simple" to parse, just that now you are doing
>>>>>>>>>> something that requires a different tool. If you have a block device,
>>>>>>>>>> you should be able to emit uevents for it, you don't need a backing
>>>>>>>>>> device, we handle virtual filesystems in /sys/block/ just fine :)
>>>>>>>>>>
>>>>>>>>>> People already have tools that listen to libudev for system monitoring
>>>>>>>>>> and management, why require them to hook up to yet-another-library? And
>>>>>>>>>> what is going to provide the ability for multiple userspace tools to
>>>>>>>>>> listen to these netlink messages in case you have more than one program
>>>>>>>>>> that wants to watch for these things (i.e. multiple desktop filesystem
>>>>>>>>>> monitoring tools, system-health checkers, etc.)?
>>>>>>>>> As much as I understand your concerns I'm not convinced uevent interface
>>>>>>>>> is a good fit. There are filesystems that don't have underlying block
>>>>>>>>> device - think of e.g. tmpfs or filesystems working directly on top of
>>>>>>>>> flash devices. These still want to send notification to userspace (one of
>>>>>>>>> primary motivation for this interfaces was so that tmpfs can notify about
>>>>>>>>> something). And creating some fake nodes in /sys/block for tmpfs and
>>>>>>>>> similar filesystems seems like doing more harm than good to me...
>>>>>>>>
>>>>>>>> If these are "fake" block devices, what's going to be present in the
>>>>>>>> block major/minor fields of the netlink message? For some reason I
>>>>>>>> thought it was a required field, and because of that, I thought we had a
>>>>>>>> "real" filesystem somewhere to refer to, otherwise how would userspace
>>>>>>>> know what filesystem was creating these events?
>>>>>>>>
>>>>>>>> What am I missing here?
>>>>>>>>
>>>>>>>> confused,
>>>>>>>>
>>>>>>>> greg k-h
>>>>>>>>
>>>>>>>
>>>>>>> For those 'fake' block devs, upon mount, get_anon_bdev will assign
>>>>>>> the major:minor numbers. Userspace might get those through stat.
>>>>>>
>>>>>> How can userspace do the mapping backwards from this "anonymous"
>>>>>> major:minor number for these types of filesystems in such a way that
>>>>>> they can "know" how to report the block device that is causing the
>>>>>> event?
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> greg k-h
>>>>>>
>>>>>
>>>>> It needs to be done internally by the app but is doable.
>>>>> The app knows what it is watching, so it can maintain the mappings.
>>>>> So prior to activating the notifications it can call 'stat' on the mount point.
>>>>> Stat struct gives the 'st_dev' which is the device id. Same will be reported
>>>>> within the message payload (through major:minor numbers). So having this,
>>>>> the app is able to get any other information it needs.
>>>>> Note that the events refer to the file system as a whole and they may not
>>>>> necessarily have anything to do with the actual block device.
>>>
>>> How are you going to show an event for a filesystem that is made up of
>>> multiple block devices?
>>>
>>>> Or you can use /proc/self/mountinfo for the mapping. There you can see
>>>> device numbers, real device names if applicable and mountpoints. This has
>>>> the advantage that it works even if filesystem mountpoints change.
>>>
>>> Ok, then that brings up my next question, how does this handle
>>> namespaces? What namespace is the event being sent in? block devices
>>> aren't namespaced, but the mount points are, is that going to cause
>>> problems?
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>
>> Getting back to the namespaces ...
>> In the current state the notifications will be sent to the init network namespace,
>> which means that processes belonging to a different net namespace will not
>> be able to receive them. To be more precise, those processes will not be
>> able to subscribe to the multicast group, though this can be easily changed.
>> Furthermore, the notifications might also be sent to specific namespace.
>> In this case, the one, with which the trace for the mount point has been registered,
>> which as I believe would be the best approach.
>>
>> As for the mount namespaces, reading the config file needs to be slightly tweaked,
>> to hide away all the registered mount points which does not belong to the current
>> mount namespace.
>>
>> Still, there is one possible 'issue' - the private/slave mount points.
>> As the notifications will be sent to all the listeners (within the same netns),
>> the events might be visible to processes outside the given mount ns.
>> This should be limited to only those listeners that share the mount namespace,
>> to which such private/slave mount points belong. As using the generic netlink
>> to filter the outgoing messages is doable (with small changes to current
>> implementation), the filters themselves seem rather cumbersome, as they would require
>> finding the socket’s owner mount namespace, which just doesn't seems right.
>> On the other hand, identifying the file system, which generated the event, will
>> not be possible for processes outside such namespace, as device major:minor
>> numbers are not bound to any namespace (afaict) so they will not provide any
>> valid information. They will remain unresolved.
>>
>> The best way out here though, is to leave it to userspace to properly setup new namespaces:
>> the mount namespace with possible private/slave mounts should have a separate
>> network namespace to isolate the potential fs events, if required.
>>
>>
>> BR
>> Beata
>>
>>
>>
>
> I'm not really sure where we are with this RFC now (?).
> Just wanted to let You know I won't be available for the next two weeks,
> in case this comes around.
>
> Best Regards
> Beata
>
>
Things has gone a bit quiet thread wise ...
As I believe I've managed to snap back to reality, I was hoping we could continue with this?
I'm not sure if we've got everything cleared up or ... have we reached a dead end?
Please let me know if we can move to the next stage? Or, if there are any showstoppers?
Thank You,
Best Regards
Beata
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Lukasz Pawelczyk @ 2015-05-26 16:42 UTC (permalink / raw)
To: Stephen Smalley
Cc: Lukasz Pawelczyk, linux-doc-u79uwXL29TY76Z2rM5mHXA, Tetsuo Handa,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Paul Moore,
Jonathan Corbet, Jingoo Han, Jeff Layton, Alexey Dobriyan,
Kees Cook, Mauro Carvalho Chehab, Fabian Frederick,
Casey Schaufler, Al Viro, James Morris, Kirill A. Shutemov,
John Johansen, Rafal Krypa, Greg KH,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Miklos Szeredi, Oleg Nesterov, Andy
In-Reply-To: <5564A088.4040507-+05T5uksL2qpZYMLLGbcSA@public.gmane.org>
On wto, 2015-05-26 at 12:34 -0400, Stephen Smalley wrote:
> > On wto, 2015-05-26 at 10:35 -0400, Stephen Smalley wrote:
> >> On 05/25/2015 08:32 AM, Lukasz Pawelczyk wrote:
> >
> > I call the inode operation by hand in the post_setxattr.
> >
> > The label will effectively be set twice, which is not ideal, but there
> > is no other option right now without reworking the hooks as you said.
> >
> > This shouldn't really be a problem because the Smack operations will not
> > use the filesystem label (even when it's set incorrectly for a moment)
> > but an already initialized smack_known structure for this inode that has
> > all the values filled in properly.
> >
> > The only attack vector I can think of is hard rebooting the machine in a
> > way that mapped label is really saved in the filesystem before the
> > unmapped will have a chance. Should I be worried about that? This sounds
> > a little unreal.
>
> If it were my security module, I would be worried about it. Even aside
> from maliciously induced failure, you are leaving yourself open to
> inconsistencies arising upon crashes. I would suggest modifying the
> setxattr hook so that the security module can override the original
> value/size pair with its own definition before it is passed to the inode
> operation. There is already precedent in that security modules are
> allowed to override the value/size returned by getxattr for security.*,
> so this just makes them fully parallel.
Will do. Thank you.
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v5 00/11] Add simple NVMEM Framework via regmap.
From: Pantelis Antoniou @ 2015-05-26 17:54 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
Rob Herring, Kumar Gala, Mark Brown, Sascha Hauer,
Greg Kroah-Hartman, linux-api-u79uwXL29TY76Z2rM5mHXA,
Linux Kernel Mailing List, devicetree,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, Matt Porter
In-Reply-To: <556438FF.7020105-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Hi Srinivas,
> On May 26, 2015, at 12:12 , Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
>
> Hi Pantelis,
>
> On 25/05/15 17:51, Pantelis Antoniou wrote:
>> Hi Srinivas,
>>
>>> On May 21, 2015, at 19:42 , Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
>>>
>>> Thankyou all for providing inputs and comments on previous versions of this patchset.
>>> Here is the v5 of the patchset addressing all the issues raised as
>>> part of previous versions review.
>>>
>>
>>>
>>
>> [snip]
>>
>> I tried to use the updated patchset with my at24 & beaglebone capemanager patches.
> Thanks for trying it out and migrating at24 to it.
>
Don’t mention it…
>>
>> I have a big problem with the removal of the raw of_* access APIs.
> Ok,
>>
>> Take for instance the case where you have multiple slot accessing different EEPROMs.
>>
>>> slots {
>>> slot@0 {
>>> eeprom = <&cape0_data>;
>>> };
>>>
>>> slot@1 {
>>> eeprom = <&cape1_data>;
>>> };
>>> };
>
> Can I ask you why should the slots be in sub-nodes?
> Do you expect to have more properties associated with each slot in future?
> Or is it just to get hold of eeprom data?
>
For now I don’t have any more properties besides the eeprom phandle.
I’ve reworked capemanager to work with the API as it is, but it’s not very intuitive IMHO.
The problem is that I have both the baseboard and the slot eeproms in a single property list.
If more per-slot properties are required, I’ll have to add again the slots node and
then the nvmem eeprom handles would stick out like a sore thumb.
>>
>> In that case there is no per-device node mapping; it’s a per-sub node.
>>
>> For now I’m exporting the of_* accessors again, please consider exposing the of_* API again.
> Sure, we can export of_nvmem_cell_get symbol for usecases like this.
>
> Having said that, I got one comment on the way the nvmem is used in your case. You should try to use nvmem_device_get() and then use nvmem_device_read() apis, These apis are for consumers like this one. The advantage of this would be you do not need read and store all data in the driver and parse them internally. Basically your ee_field_get would just do nvmem_device_read(); Does it make sense?
>
Hmm, good idea; I’ll give it a shot.
> We can work on how to get the of_*based once you decide to move to this api.
>
>
OK, thanks a lot for taking the time to think about this Srinivas.
>
> --srini
>>
Regards
— Pantelis
>>> --
>>> 1.9.1
>>>
>>
>> Regards
>>
>> — Pantelis
>>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH] percpu system call: fast userspace percpu critical sections
From: Andy Lutomirski @ 2015-05-26 19:57 UTC (permalink / raw)
To: Andi Kleen, Mathieu Desnoyers, Borislav Petkov, H. Peter Anvin
Cc: Lai Jiangshan, Ben Maurer, Paul E. McKenney, Ingo Molnar,
Josh Triplett, Andrew Morton, Michael Kerrisk, Linux API,
Linux Kernel, Paul Turner, Peter Zijlstra, Linus Torvalds,
Steven Rostedt, Andrew Hunter
In-Reply-To: <CALCETrW3_Hv0jc3cpiwsHTinBqJzvab_EiPS8BVJhX-xe5D8qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On May 25, 2015 11:54 AM, "Andy Lutomirski" <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>
> [cc: hpa, Borislav and Andi]
>
> On Mon, May 25, 2015 at 11:30 AM, Mathieu Desnoyers
> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> > ----- Original Message -----
> >> On May 23, 2015 10:09 AM, "Mathieu Desnoyers"
> >> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> >> >
> >> > ----- Original Message -----
> >> > > On Fri, May 22, 2015 at 2:34 PM, Mathieu Desnoyers
> >> > > <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> >> > > > ----- Original Message -----
> >> > > >> On Fri, May 22, 2015 at 1:26 PM, Michael Kerrisk
> >> > > >> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> > > >> wrote:
> >> > > >> > [CC += linux-api@]
> >> > > >> >
> >> > > >> > On Thu, May 21, 2015 at 4:44 PM, Mathieu Desnoyers
> >> > > >> > <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> >> > > >> >> Expose a new system call allowing userspace threads to register
> >> > > >> >> a TLS area used as an ABI between the kernel and userspace to
> >> > > >> >> share information required to create efficient per-cpu critical
> >> > > >> >> sections in user-space.
> >> > > >> >>
> >> > > >> >> This ABI consists of a thread-local structure containing:
> >> > > >> >>
> >> > > >> >> - a nesting count surrounding the critical section,
> >> > > >> >> - a signal number to be sent to the thread when preempting a thread
> >> > > >> >> with non-zero nesting count,
> >> > > >> >> - a flag indicating whether the signal has been sent within the
> >> > > >> >> critical section,
> >> > > >> >> - an integer where to store the current CPU number, updated
> >> > > >> >> whenever
> >> > > >> >> the thread is preempted. This CPU number cache is not strictly
> >> > > >> >> needed, but performs better than getcpu vdso.
> >> > > >> >>
> >> > > >> >> This approach is inspired by Paul Turner and Andrew Hunter's work
> >> > > >> >> on percpu atomics, which lets the kernel handle restart of critical
> >> > > >> >> sections, ref.
> >> > > >> >> http://www.linuxplumbersconf.org/2013/ocw/system/presentations/1695/original/LPC%20-%20PerCpu%20Atomics.pdf
> >> > > >> >>
> >> > > >> >> What is done differently here compared to percpu atomics: we track
> >> > > >> >> a single nesting counter per thread rather than many ranges of
> >> > > >> >> instruction pointer values. We deliver a signal to user-space and
> >> > > >> >> let the logic of restart be handled in user-space, thus moving
> >> > > >> >> the complexity out of the kernel. The nesting counter approach
> >> > > >> >> allows us to skip the complexity of interacting with signals that
> >> > > >> >> would be otherwise needed with the percpu atomics approach, which
> >> > > >> >> needs to know which instruction pointers are preempted, including
> >> > > >> >> when preemption occurs on a signal handler nested over an
> >> > > >> >> instruction
> >> > > >> >> pointer of interest.
> >> > > >> >>
> >> > > >>
> >> > > >> I talked about this kind of thing with PeterZ at LSF/MM, and I was
> >> > > >> unable to convince myself that the kernel needs to help at all. To do
> >> > > >> this without kernel help, I want to relax the requirements slightly.
> >> > > >> With true per-cpu atomic sections, you have a guarantee that you are
> >> > > >> either really running on the same CPU for the entire duration of the
> >> > > >> atomic section or you abort. I propose a weaker primitive: you
> >> > > >> acquire one of an array of locks (probably one per cpu), and you are
> >> > > >> guaranteed that, if you don't abort, no one else acquires the same
> >> > > >> lock while you hold it.
> >> > > >
> >> > > > In my proof of concept (https://github.com/compudj/percpu-dev) I
> >> > > > actually implement an array of per-cpu lock. The issue here boils
> >> > > > down to grabbing this per-cpu lock efficiently. Once the lock is taken,
> >> > > > the thread has exclusive access to that per-cpu lock, even if it
> >> > > > migrates.
> >> > > >
> >> > > >> Here's how:
> >> > > >>
> >> > > >> Create an array of user-managed locks, one per cpu. Call them lock[i]
> >> > > >> for 0 <= i < ncpus.
> >> > > >>
> >> > > >> To acquire, look up your CPU number. Then, atomically, check that
> >> > > >> lock[cpu] isn't held and, if so, mark it held and record both your tid
> >> > > >> and your lock acquisition count. If you learn that the lock *was*
> >> > > >> held after all, signal the holder (with kill or your favorite other
> >> > > >> mechanism), telling it which lock acquisition count is being aborted.
> >> > > >> Then atomically steal the lock, but only if the lock acquisition count
> >> > > >> hasn't changed.
> >> > > >>
> >> > > >> This has a few benefits over the in-kernel approach:
> >> > > >>
> >> > > >> 1. No kernel patch.
> >> > > >>
> >> > > >> 2. No unnecessary abort if you are preempted in favor of a thread that
> >> > > >> doesn't content for your lock.
> >> > > >>
> >> > > >> 3. Greatly improved debuggability.
> >> > > >>
> >> > > >> 4. With long critical sections and heavy load, you can improve
> >> > > >> performance by having several locks per cpu and choosing one at
> >> > > >> random.
> >> > > >>
> >> > > >> Is there a reason that a scheme like this doesn't work?
> >> > > >
> >> > > > What do you mean exactly by "atomically check that lock is not
> >> > > > held and, if so, mark it held" ? Do you imply using a lock-prefixed
> >> > > > atomic operation ?
> >> > >
> >> > > Yes.
> >> > >
> >> > > >
> >> > > > The goal of this whole restart section approach is to allow grabbing
> >> > > > a lock (or doing other sequences of operations ending with a single
> >> > > > store) on per-cpu data without having to use slow lock-prefixed
> >> > > > atomic operations.
> >> > >
> >> > > Ah, ok, I assumed it was to allow multiple threads to work in parallel.
> >> > >
> >> > > How arch-specific are you willing to be?
> >> >
> >> > I'd want this to be usable on every major architectures.
> >> >
> >> > > On x86, it might be possible
> >> > > to play some GDT games so that an unlocked xchg relative
> >> >
> >> > AFAIK, there is no such thing as an unlocked xchg. xchg always
> >> > imply the lock prefix on x86. I guess you mean cmpxchg here.
> >> >
> >>
> >> Right, got my special cases mixed up.
> >>
> >> I wonder if we could instead have a vdso function that did something like:
> >>
> >> unsigned long __vdso_cpu_local_exchange(unsigned long *base, int
> >> shift, unsigned long newval)
> >> {
> >> unsigned long *ptr = base + (cpu << shift);
> >> unsigned long old = *ptr;
> >> *ptr = new;
> >> return *ptr;
> >> }
> >>
> >> I think this primitive would be sufficient to let user code do the
> >> rest. There might be other more simple primitives that would work.
> >> It could be implemented by fiddling with IP ranges, but we could
> >> change the implementation later without breaking anything. The only
> >> really hard part would be efficiently figuring out what CPU we're on.
> >
> > The "fiddling with IP ranges" is where the restart sections come into
> > play. Paul Turner's approach indeed knows about IP ranges, and performs
> > the restart from the kernel. My alternative approach uses a signal and
> > page protection in user-space to reach the same result. It appears that
> > CONFIG_PREEMPT kernels are difficult to handle with Paul's approach, so
> > perhaps we could combine our approaches to get the best of both.
>
> I'm not sure why CONFIG_PREEMPT would matter. What am I missing?
>
> Doing this in the vdso has some sneaky benefits: rather than aborting
> a very short vdso-based primitive on context switch, we could just fix
> it up in the kernel and skip ahead to the end.
I might be guilty of being too x86-centric here. On x86, as long as
the lock and unlock primitives are sufficiently atomic, everything
should be okay. On other architectures, though, a primitive that
gives lock, unlock, and abort of a per-cpu lock without checking that
you're still on the right cpu at unlock time may not be sufficient.
If the primitive is implemented purely with loads and stores, then
even if you take the lock, migrate, finish your work, and unlock
without anyone else contending from the lock (and hence don't abort),
the next thread to take the same lock will end up unsynchronized
unless there's appropriate memory ordering. For example, if taking
the lock were an acquire and unlocking were a release, we'd be fine.
Your RFC design certainly works (in principle -- I haven't looked at
the code in detail), but I can't shake the feeling that it's overkill
and that it could be improved to avoid unnecessary aborts every time
the lock holder is scheduled out.
This isn't a problem in your RFC design, but if we wanted to come up
with tighter primitives, we'd have to be quite careful to document
exactly what memory ordering guarantees they come with.
It may be that all architectures for which you care about the
performance boost already have efficient acquire and release
operations. Certainly x86 does, and I don't know how fast the new ARM
instructions are, but I imagine they're pretty good.
It's too bad that not all architectures have a single-instruction
unlocked compare-and-exchange.
--Andy
^ permalink raw reply
* Re: [RFC PATCH] percpu system call: fast userspace percpu critical sections
From: Mathieu Desnoyers @ 2015-05-26 20:38 UTC (permalink / raw)
To: Andy Lutomirski
Cc: H. Peter Anvin, Andi Kleen, Borislav Petkov, Lai Jiangshan,
Paul E. McKenney, Ben Maurer, Josh Triplett, Ingo Molnar,
Andrew Morton, Linux API, Michael Kerrisk, Linux Kernel,
Linus Torvalds, Peter Zijlstra, Paul Turner, Steven Rostedt,
Andrew Hunter
In-Reply-To: <CALCETrW3_Hv0jc3cpiwsHTinBqJzvab_EiPS8BVJhX-xe5D8qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
----- Original Message -----
> [cc: hpa, Borislav and Andi]
>
> On Mon, May 25, 2015 at 11:30 AM, Mathieu Desnoyers
> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> > ----- Original Message -----
> >> On May 23, 2015 10:09 AM, "Mathieu Desnoyers"
> >> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> >> >
> >> > ----- Original Message -----
> >> > > On Fri, May 22, 2015 at 2:34 PM, Mathieu Desnoyers
> >> > > <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> >> > > > ----- Original Message -----
> >> > > >> On Fri, May 22, 2015 at 1:26 PM, Michael Kerrisk
> >> > > >> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> > > >> wrote:
> >> > > >> > [CC += linux-api@]
> >> > > >> >
> >> > > >> > On Thu, May 21, 2015 at 4:44 PM, Mathieu Desnoyers
> >> > > >> > <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> >> > > >> >> Expose a new system call allowing userspace threads to register
> >> > > >> >> a TLS area used as an ABI between the kernel and userspace to
> >> > > >> >> share information required to create efficient per-cpu critical
> >> > > >> >> sections in user-space.
> >> > > >> >>
> >> > > >> >> This ABI consists of a thread-local structure containing:
> >> > > >> >>
> >> > > >> >> - a nesting count surrounding the critical section,
> >> > > >> >> - a signal number to be sent to the thread when preempting a
> >> > > >> >> thread
> >> > > >> >> with non-zero nesting count,
> >> > > >> >> - a flag indicating whether the signal has been sent within the
> >> > > >> >> critical section,
> >> > > >> >> - an integer where to store the current CPU number, updated
> >> > > >> >> whenever
> >> > > >> >> the thread is preempted. This CPU number cache is not strictly
> >> > > >> >> needed, but performs better than getcpu vdso.
> >> > > >> >>
> >> > > >> >> This approach is inspired by Paul Turner and Andrew Hunter's
> >> > > >> >> work
> >> > > >> >> on percpu atomics, which lets the kernel handle restart of
> >> > > >> >> critical
> >> > > >> >> sections, ref.
> >> > > >> >> http://www.linuxplumbersconf.org/2013/ocw/system/presentations/1695/original/LPC%20-%20PerCpu%20Atomics.pdf
> >> > > >> >>
> >> > > >> >> What is done differently here compared to percpu atomics: we
> >> > > >> >> track
> >> > > >> >> a single nesting counter per thread rather than many ranges of
> >> > > >> >> instruction pointer values. We deliver a signal to user-space
> >> > > >> >> and
> >> > > >> >> let the logic of restart be handled in user-space, thus moving
> >> > > >> >> the complexity out of the kernel. The nesting counter approach
> >> > > >> >> allows us to skip the complexity of interacting with signals
> >> > > >> >> that
> >> > > >> >> would be otherwise needed with the percpu atomics approach,
> >> > > >> >> which
> >> > > >> >> needs to know which instruction pointers are preempted,
> >> > > >> >> including
> >> > > >> >> when preemption occurs on a signal handler nested over an
> >> > > >> >> instruction
> >> > > >> >> pointer of interest.
> >> > > >> >>
> >> > > >>
> >> > > >> I talked about this kind of thing with PeterZ at LSF/MM, and I was
> >> > > >> unable to convince myself that the kernel needs to help at all. To
> >> > > >> do
> >> > > >> this without kernel help, I want to relax the requirements
> >> > > >> slightly.
> >> > > >> With true per-cpu atomic sections, you have a guarantee that you
> >> > > >> are
> >> > > >> either really running on the same CPU for the entire duration of
> >> > > >> the
> >> > > >> atomic section or you abort. I propose a weaker primitive: you
> >> > > >> acquire one of an array of locks (probably one per cpu), and you
> >> > > >> are
> >> > > >> guaranteed that, if you don't abort, no one else acquires the same
> >> > > >> lock while you hold it.
> >> > > >
> >> > > > In my proof of concept (https://github.com/compudj/percpu-dev) I
> >> > > > actually implement an array of per-cpu lock. The issue here boils
> >> > > > down to grabbing this per-cpu lock efficiently. Once the lock is
> >> > > > taken,
> >> > > > the thread has exclusive access to that per-cpu lock, even if it
> >> > > > migrates.
> >> > > >
> >> > > >> Here's how:
> >> > > >>
> >> > > >> Create an array of user-managed locks, one per cpu. Call them
> >> > > >> lock[i]
> >> > > >> for 0 <= i < ncpus.
> >> > > >>
> >> > > >> To acquire, look up your CPU number. Then, atomically, check that
> >> > > >> lock[cpu] isn't held and, if so, mark it held and record both your
> >> > > >> tid
> >> > > >> and your lock acquisition count. If you learn that the lock *was*
> >> > > >> held after all, signal the holder (with kill or your favorite other
> >> > > >> mechanism), telling it which lock acquisition count is being
> >> > > >> aborted.
> >> > > >> Then atomically steal the lock, but only if the lock acquisition
> >> > > >> count
> >> > > >> hasn't changed.
> >> > > >>
> >> > > >> This has a few benefits over the in-kernel approach:
> >> > > >>
> >> > > >> 1. No kernel patch.
> >> > > >>
> >> > > >> 2. No unnecessary abort if you are preempted in favor of a thread
> >> > > >> that
> >> > > >> doesn't content for your lock.
> >> > > >>
> >> > > >> 3. Greatly improved debuggability.
> >> > > >>
> >> > > >> 4. With long critical sections and heavy load, you can improve
> >> > > >> performance by having several locks per cpu and choosing one at
> >> > > >> random.
> >> > > >>
> >> > > >> Is there a reason that a scheme like this doesn't work?
> >> > > >
> >> > > > What do you mean exactly by "atomically check that lock is not
> >> > > > held and, if so, mark it held" ? Do you imply using a lock-prefixed
> >> > > > atomic operation ?
> >> > >
> >> > > Yes.
> >> > >
> >> > > >
> >> > > > The goal of this whole restart section approach is to allow grabbing
> >> > > > a lock (or doing other sequences of operations ending with a single
> >> > > > store) on per-cpu data without having to use slow lock-prefixed
> >> > > > atomic operations.
> >> > >
> >> > > Ah, ok, I assumed it was to allow multiple threads to work in
> >> > > parallel.
> >> > >
> >> > > How arch-specific are you willing to be?
> >> >
> >> > I'd want this to be usable on every major architectures.
> >> >
> >> > > On x86, it might be possible
> >> > > to play some GDT games so that an unlocked xchg relative
> >> >
> >> > AFAIK, there is no such thing as an unlocked xchg. xchg always
> >> > imply the lock prefix on x86. I guess you mean cmpxchg here.
> >> >
> >>
> >> Right, got my special cases mixed up.
> >>
> >> I wonder if we could instead have a vdso function that did something like:
> >>
> >> unsigned long __vdso_cpu_local_exchange(unsigned long *base, int
> >> shift, unsigned long newval)
> >> {
> >> unsigned long *ptr = base + (cpu << shift);
> >> unsigned long old = *ptr;
> >> *ptr = new;
> >> return *ptr;
> >> }
> >>
> >> I think this primitive would be sufficient to let user code do the
> >> rest. There might be other more simple primitives that would work.
> >> It could be implemented by fiddling with IP ranges, but we could
> >> change the implementation later without breaking anything. The only
> >> really hard part would be efficiently figuring out what CPU we're on.
> >
> > The "fiddling with IP ranges" is where the restart sections come into
> > play. Paul Turner's approach indeed knows about IP ranges, and performs
> > the restart from the kernel. My alternative approach uses a signal and
> > page protection in user-space to reach the same result. It appears that
> > CONFIG_PREEMPT kernels are difficult to handle with Paul's approach, so
> > perhaps we could combine our approaches to get the best of both.
>
> I'm not sure why CONFIG_PREEMPT would matter. What am I missing?
With CONFIG_PREEMPT, the scheduler can preempt the kernel while
it's running a page fault, or a system call, on behalf of a thread.
So in addition of having to check which instruction pointer is preempted,
and which IP is having a signal delivered on, we may need to add a check
when entering into the kernel on pretty much every path, including
page faults and system calls. I presume the interrupt handler is OK,
provided that interrupt handlers cannot be preempted, and that the explicit
preempt check is done before the interrupt handler returns to user-space,
AFAIU passing the user-space pt_regs, which should be OK already.
I may very have missed other subtleties of issues related to pjt's approach
with CONFIG_PREEMPT. Hopefully he can enlighten us with the missing parts.
>
> Doing this in the vdso has some sneaky benefits: rather than aborting
> a very short vdso-based primitive on context switch, we could just fix
> it up in the kernel and skip ahead to the end.
The fixup rather than restart idea is quite interesting. However, it may
require to do the fixup in the kernel before being migrated, and I'm not
sure it's that easy to find a preemptable spot where we can do the user
accesses needed for the fixup before migration. One advantage of the
"restart" approach is that we can perform the user-accesses after the
migration, which allow us to call that code right before return to
userspace. Unfortunately, doing the fixup from the kernel after the
migration might be tricky, even if we use a lock-atomic op in the fixup,
because we might be doing a store concurrently with a non-locked atomic
op running in userspace on the original CPU.
Here is an idea I'm not totally proud of, but might work if we really
want to do this without restarting userspace: we could expose a vdso
that both performs an atomic operation on an array of pointers to
per-cpu values, and gets the cpu number:
void __vdso_cmpxchg_getcpu(unsigned long **p, unsigned long _old,
unsigned long _new, int *cpu);
If we preempt this vdso, the in-kernel fixup simply grabs the current CPU
number and updates the right array entry, all this locally. The advantage
here is that we can perform this after migration. It comes at the expense
of an indirection through the pointer array, which I rather dislike. If
we can rather find a way to perform the userspace fixup before migration,
it would be much better.
>
> >
> >>
> >> FWIW, 'xchg' to cache-hot memory is only about 20 cycles on my laptop,
> >> and cmpxchg seems to be about 6 cycles. Both are faster than getcpu.
> >> How much are we really saving with any of this over the pure userspace
> >> approach? I think that the most that the kernel can possibly help us
> >> is to give us a faster getcpu and to help us deal with being migrated
> >> to a different cpu if we want to avoid expensive operations and don't
> >> want to play unlocked cmpxchg tricks.
> >
> > You appear to summarize the two things that are added to the kernel with
> > this RFC patch:
> > - faster getcpu(): 12.7 ns -> 0.3 ns
>
> Yeah, I figured that out the second time I read your email and re-read
> the original message, but I apparently forgot to fix up the email I
> was typing.
>
> > - allow using less expensive operations to perform per-cpu-atomic ops:
> > (on my system):
> > - lock; cmpxchg (18 ns) -> load-test-branch-store (5.4 ns)
> >
>
> When you say "load-test-branch-store", do you mean that sequence of
> normal code or a literal cmpxchg? On x86, we really can pull off the
> sneaky trick of gs-relative cmpxchg in a single instruction, which
> gives us per-cpu atomic locking without any special kernel fixups,
> although that locks down gs and isn't currently supported.
It's a sequence of normal code. Ideally I'd want this to be doable
on other architectures too (powerpc and ARM at least).
>
> IIRC some other OS's use gs as a userspace per-cpu pointer instead of
> a per-thread pointer. Would we want to consider enabling that?
> This may interact oddly with the WIP wrgsbase stuff. (Grr, Intel, why
> couldn't you have made wrgsbase reset the gs selector to zero? That
> would have been so much nicer.)
>
> >>
> >> I was wrong about set_thread_area giving us efficient per-cpu data,
> >> BTW, although it would be easy to add a similar feature that would
> >> give us a per-cpu segment base on x86.
> >
> > ok
> >
>
> >>
> >> This ABI is kind of unfortunate in that it can't be shared between
> >> multiple libraries.
> >
> > Well ideally you'd want one library to implement this (e.g. glibc), and
> > other libraries to use it. I'm not sure doing a linked list of TLS entries
> > per thread would really be useful here.
>
> True, but that may not play so well with tracing injection.
>
> At least a pure vdso approach is guaranteed not to have this problem.
Yep.
Thanks!
Mathieu
>
>
> >> Oh, you're delivering a signal every time you're preempted, even
> >> outside the critical section? That sounds expensive.
> >
> > No, the signal is only delivered if the kernel preempts the critical
> > section.
>
> As noted above, I meant to delete this before sending :)
>
> --Andy
>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH] percpu system call: fast userspace percpu critical sections
From: Andy Lutomirski @ 2015-05-26 20:58 UTC (permalink / raw)
To: Mathieu Desnoyers, Konstantin Khlebnikov
Cc: H. Peter Anvin, Andi Kleen, Borislav Petkov, Lai Jiangshan,
Paul E. McKenney, Ben Maurer, Josh Triplett, Ingo Molnar,
Andrew Morton, Linux API, Michael Kerrisk, Linux Kernel,
Linus Torvalds, Peter Zijlstra, Paul Turner, Steven Rostedt,
Andrew Hunter
In-Reply-To: <933886515.8478.1432672739485.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On Tue, May 26, 2015 at 1:38 PM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> ----- Original Message -----
>> [cc: hpa, Borislav and Andi]
>>
>> On Mon, May 25, 2015 at 11:30 AM, Mathieu Desnoyers
>> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
>> > ----- Original Message -----
>> >> On May 23, 2015 10:09 AM, "Mathieu Desnoyers"
>> >> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
>> >> >
>> >> > ----- Original Message -----
>> >> > > On Fri, May 22, 2015 at 2:34 PM, Mathieu Desnoyers
>> >> > > <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
>> >> > > > ----- Original Message -----
>> >> > > >> On Fri, May 22, 2015 at 1:26 PM, Michael Kerrisk
>> >> > > >> <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >> > > >> wrote:
>> >> > > >> > [CC += linux-api@]
>> >> > > >> >
>> >> > > >> > On Thu, May 21, 2015 at 4:44 PM, Mathieu Desnoyers
>> >> > > >> > <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
>> >> > > >> >> Expose a new system call allowing userspace threads to register
>> >> > > >> >> a TLS area used as an ABI between the kernel and userspace to
>> >> > > >> >> share information required to create efficient per-cpu critical
>> >> > > >> >> sections in user-space.
>> >> > > >> >>
>> >> > > >> >> This ABI consists of a thread-local structure containing:
>> >> > > >> >>
>> >> > > >> >> - a nesting count surrounding the critical section,
>> >> > > >> >> - a signal number to be sent to the thread when preempting a
>> >> > > >> >> thread
>> >> > > >> >> with non-zero nesting count,
>> >> > > >> >> - a flag indicating whether the signal has been sent within the
>> >> > > >> >> critical section,
>> >> > > >> >> - an integer where to store the current CPU number, updated
>> >> > > >> >> whenever
>> >> > > >> >> the thread is preempted. This CPU number cache is not strictly
>> >> > > >> >> needed, but performs better than getcpu vdso.
>> >> > > >> >>
>> >> > > >> >> This approach is inspired by Paul Turner and Andrew Hunter's
>> >> > > >> >> work
>> >> > > >> >> on percpu atomics, which lets the kernel handle restart of
>> >> > > >> >> critical
>> >> > > >> >> sections, ref.
>> >> > > >> >> http://www.linuxplumbersconf.org/2013/ocw/system/presentations/1695/original/LPC%20-%20PerCpu%20Atomics.pdf
>> >> > > >> >>
>> >> > > >> >> What is done differently here compared to percpu atomics: we
>> >> > > >> >> track
>> >> > > >> >> a single nesting counter per thread rather than many ranges of
>> >> > > >> >> instruction pointer values. We deliver a signal to user-space
>> >> > > >> >> and
>> >> > > >> >> let the logic of restart be handled in user-space, thus moving
>> >> > > >> >> the complexity out of the kernel. The nesting counter approach
>> >> > > >> >> allows us to skip the complexity of interacting with signals
>> >> > > >> >> that
>> >> > > >> >> would be otherwise needed with the percpu atomics approach,
>> >> > > >> >> which
>> >> > > >> >> needs to know which instruction pointers are preempted,
>> >> > > >> >> including
>> >> > > >> >> when preemption occurs on a signal handler nested over an
>> >> > > >> >> instruction
>> >> > > >> >> pointer of interest.
>> >> > > >> >>
>> >> > > >>
>> >> > > >> I talked about this kind of thing with PeterZ at LSF/MM, and I was
>> >> > > >> unable to convince myself that the kernel needs to help at all. To
>> >> > > >> do
>> >> > > >> this without kernel help, I want to relax the requirements
>> >> > > >> slightly.
>> >> > > >> With true per-cpu atomic sections, you have a guarantee that you
>> >> > > >> are
>> >> > > >> either really running on the same CPU for the entire duration of
>> >> > > >> the
>> >> > > >> atomic section or you abort. I propose a weaker primitive: you
>> >> > > >> acquire one of an array of locks (probably one per cpu), and you
>> >> > > >> are
>> >> > > >> guaranteed that, if you don't abort, no one else acquires the same
>> >> > > >> lock while you hold it.
>> >> > > >
>> >> > > > In my proof of concept (https://github.com/compudj/percpu-dev) I
>> >> > > > actually implement an array of per-cpu lock. The issue here boils
>> >> > > > down to grabbing this per-cpu lock efficiently. Once the lock is
>> >> > > > taken,
>> >> > > > the thread has exclusive access to that per-cpu lock, even if it
>> >> > > > migrates.
>> >> > > >
>> >> > > >> Here's how:
>> >> > > >>
>> >> > > >> Create an array of user-managed locks, one per cpu. Call them
>> >> > > >> lock[i]
>> >> > > >> for 0 <= i < ncpus.
>> >> > > >>
>> >> > > >> To acquire, look up your CPU number. Then, atomically, check that
>> >> > > >> lock[cpu] isn't held and, if so, mark it held and record both your
>> >> > > >> tid
>> >> > > >> and your lock acquisition count. If you learn that the lock *was*
>> >> > > >> held after all, signal the holder (with kill or your favorite other
>> >> > > >> mechanism), telling it which lock acquisition count is being
>> >> > > >> aborted.
>> >> > > >> Then atomically steal the lock, but only if the lock acquisition
>> >> > > >> count
>> >> > > >> hasn't changed.
>> >> > > >>
>> >> > > >> This has a few benefits over the in-kernel approach:
>> >> > > >>
>> >> > > >> 1. No kernel patch.
>> >> > > >>
>> >> > > >> 2. No unnecessary abort if you are preempted in favor of a thread
>> >> > > >> that
>> >> > > >> doesn't content for your lock.
>> >> > > >>
>> >> > > >> 3. Greatly improved debuggability.
>> >> > > >>
>> >> > > >> 4. With long critical sections and heavy load, you can improve
>> >> > > >> performance by having several locks per cpu and choosing one at
>> >> > > >> random.
>> >> > > >>
>> >> > > >> Is there a reason that a scheme like this doesn't work?
>> >> > > >
>> >> > > > What do you mean exactly by "atomically check that lock is not
>> >> > > > held and, if so, mark it held" ? Do you imply using a lock-prefixed
>> >> > > > atomic operation ?
>> >> > >
>> >> > > Yes.
>> >> > >
>> >> > > >
>> >> > > > The goal of this whole restart section approach is to allow grabbing
>> >> > > > a lock (or doing other sequences of operations ending with a single
>> >> > > > store) on per-cpu data without having to use slow lock-prefixed
>> >> > > > atomic operations.
>> >> > >
>> >> > > Ah, ok, I assumed it was to allow multiple threads to work in
>> >> > > parallel.
>> >> > >
>> >> > > How arch-specific are you willing to be?
>> >> >
>> >> > I'd want this to be usable on every major architectures.
>> >> >
>> >> > > On x86, it might be possible
>> >> > > to play some GDT games so that an unlocked xchg relative
>> >> >
>> >> > AFAIK, there is no such thing as an unlocked xchg. xchg always
>> >> > imply the lock prefix on x86. I guess you mean cmpxchg here.
>> >> >
>> >>
>> >> Right, got my special cases mixed up.
>> >>
>> >> I wonder if we could instead have a vdso function that did something like:
>> >>
>> >> unsigned long __vdso_cpu_local_exchange(unsigned long *base, int
>> >> shift, unsigned long newval)
>> >> {
>> >> unsigned long *ptr = base + (cpu << shift);
>> >> unsigned long old = *ptr;
>> >> *ptr = new;
>> >> return *ptr;
>> >> }
>> >>
>> >> I think this primitive would be sufficient to let user code do the
>> >> rest. There might be other more simple primitives that would work.
>> >> It could be implemented by fiddling with IP ranges, but we could
>> >> change the implementation later without breaking anything. The only
>> >> really hard part would be efficiently figuring out what CPU we're on.
>> >
>> > The "fiddling with IP ranges" is where the restart sections come into
>> > play. Paul Turner's approach indeed knows about IP ranges, and performs
>> > the restart from the kernel. My alternative approach uses a signal and
>> > page protection in user-space to reach the same result. It appears that
>> > CONFIG_PREEMPT kernels are difficult to handle with Paul's approach, so
>> > perhaps we could combine our approaches to get the best of both.
>>
>> I'm not sure why CONFIG_PREEMPT would matter. What am I missing?
>
> With CONFIG_PREEMPT, the scheduler can preempt the kernel while
> it's running a page fault, or a system call, on behalf of a thread.
>
> So in addition of having to check which instruction pointer is preempted,
> and which IP is having a signal delivered on, we may need to add a check
> when entering into the kernel on pretty much every path, including
> page faults and system calls. I presume the interrupt handler is OK,
> provided that interrupt handlers cannot be preempted, and that the explicit
> preempt check is done before the interrupt handler returns to user-space,
> AFAIU passing the user-space pt_regs, which should be OK already.
>
> I may very have missed other subtleties of issues related to pjt's approach
> with CONFIG_PREEMPT. Hopefully he can enlighten us with the missing parts.
Oh, right.
FWIW, we don't really have accurate tracking of the original user
state before the outermost nested entry. There's perf_get_regs_user,
but that's a hack. On the other hand, we only care about potentially
preemptable entries, so no NMI, but even machine checks can sort of
sleep these days (even on non-CONFIG_PREEMPT).
Yuck.
>
>>
>> Doing this in the vdso has some sneaky benefits: rather than aborting
>> a very short vdso-based primitive on context switch, we could just fix
>> it up in the kernel and skip ahead to the end.
>
> The fixup rather than restart idea is quite interesting. However, it may
> require to do the fixup in the kernel before being migrated, and I'm not
> sure it's that easy to find a preemptable spot where we can do the user
> accesses needed for the fixup before migration.
Isn't schedule() itself a reasonable spot? We could wrap it in
pagefault_disable() and presumably find some reasonable way to retry
if the fault fails. We also might not need user vm access at all --
pt_regs could be sufficient. But the same CONFIG_PREEMPT caveat
applies, I think.
> One advantage of the
> "restart" approach is that we can perform the user-accesses after the
> migration, which allow us to call that code right before return to
> userspace. Unfortunately, doing the fixup from the kernel after the
> migration might be tricky, even if we use a lock-atomic op in the fixup,
> because we might be doing a store concurrently with a non-locked atomic
> op running in userspace on the original CPU.
But we can check what cpu we're on if we're in the kernel. In fact,
once the x86 exit-to-userspace code gets rewritten (working on that,
but maybe not ready for 4.2), this would actually be straightforward,
modulo forcing a slower exit path on migration if we don't have a good
way to exclude migrations that don't matter.
>
> Here is an idea I'm not totally proud of, but might work if we really
> want to do this without restarting userspace: we could expose a vdso
> that both performs an atomic operation on an array of pointers to
> per-cpu values, and gets the cpu number:
>
> void __vdso_cmpxchg_getcpu(unsigned long **p, unsigned long _old,
> unsigned long _new, int *cpu);
>
> If we preempt this vdso, the in-kernel fixup simply grabs the current CPU
> number and updates the right array entry, all this locally. The advantage
> here is that we can perform this after migration. It comes at the expense
> of an indirection through the pointer array, which I rather dislike. If
> we can rather find a way to perform the userspace fixup before migration,
> it would be much better.
Sneaky. I think that's actually kind of neat, except that it's
incompatible with the x86 gs-based cmpxchg trick.
FWIW, I should probably stop complaining about having some per-process
non-library-compatible state. We already have that with
set_robust_list. If we want to have userspace register a per-thread
virtual address at which the thread's cpu number lives, so be it. How
do we handle a page fault on migration, though?
Grumble. I really wish that we had per-cpu virtual memory mappings.
Oh, well. Also, I think PeterZ's opinion on anything that mucks with
the scheduler is critical here.
>
>>
>> >
>> >>
>> >> FWIW, 'xchg' to cache-hot memory is only about 20 cycles on my laptop,
>> >> and cmpxchg seems to be about 6 cycles. Both are faster than getcpu.
>> >> How much are we really saving with any of this over the pure userspace
>> >> approach? I think that the most that the kernel can possibly help us
>> >> is to give us a faster getcpu and to help us deal with being migrated
>> >> to a different cpu if we want to avoid expensive operations and don't
>> >> want to play unlocked cmpxchg tricks.
>> >
>> > You appear to summarize the two things that are added to the kernel with
>> > this RFC patch:
>> > - faster getcpu(): 12.7 ns -> 0.3 ns
>>
>> Yeah, I figured that out the second time I read your email and re-read
>> the original message, but I apparently forgot to fix up the email I
>> was typing.
>>
>> > - allow using less expensive operations to perform per-cpu-atomic ops:
>> > (on my system):
>> > - lock; cmpxchg (18 ns) -> load-test-branch-store (5.4 ns)
>> >
>>
>> When you say "load-test-branch-store", do you mean that sequence of
>> normal code or a literal cmpxchg? On x86, we really can pull off the
>> sneaky trick of gs-relative cmpxchg in a single instruction, which
>> gives us per-cpu atomic locking without any special kernel fixups,
>> although that locks down gs and isn't currently supported.
>
> It's a sequence of normal code. Ideally I'd want this to be doable
> on other architectures too (powerpc and ARM at least).
Agreed.
BTW, I just found this thread:
http://comments.gmane.org/gmane.linux.kernel/1786674
this is kind of like my sort-of-proposal for using gs.
--Andy
^ permalink raw reply
* Re: [RFC PATCH] percpu system call: fast userspace percpu critical sections
From: Mathieu Desnoyers @ 2015-05-26 21:04 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andi Kleen, Borislav Petkov, H. Peter Anvin, Lai Jiangshan,
Ben Maurer, Paul E. McKenney, Ingo Molnar, Josh Triplett,
Andrew Morton, Michael Kerrisk, Linux API, Linux Kernel,
Paul Turner, Peter Zijlstra, Linus Torvalds, Steven Rostedt,
Andrew Hunter
In-Reply-To: <CALCETrXzmO=fQC=UdCh5b0zWiGWAJScEtdT4QDJkoqLgtgEVig-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
----- Original Message -----
> On May 25, 2015 11:54 AM, "Andy Lutomirski" <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
[...]
> I might be guilty of being too x86-centric here. On x86, as long as
> the lock and unlock primitives are sufficiently atomic, everything
> should be okay. On other architectures, though, a primitive that
> gives lock, unlock, and abort of a per-cpu lock without checking that
> you're still on the right cpu at unlock time may not be sufficient.
> If the primitive is implemented purely with loads and stores, then
> even if you take the lock, migrate, finish your work, and unlock
> without anyone else contending from the lock (and hence don't abort),
> the next thread to take the same lock will end up unsynchronized
> unless there's appropriate memory ordering. For example, if taking
> the lock were an acquire and unlocking were a release, we'd be fine.
If we look at x86-TSO, where stores reordered after loads is pretty
much the only thing we need to care about, we have:
CPU 0 CPU 1
load, test lock[1]
store lock[1]
loads/stores to data[1]
[preempted, migrate to cpu 0]
[run other thread]
load, test lock[1] (loop)
loads/stores to data[1] (A)
store 0 to lock[1] (B)
load, test lock[1] (C)
store lock[1]
loads/stores to data[1] (D)
What we want to ensure is that load/stores to data[1] from CPU 0 and
1 don't get interleaved.
On CPU 0 doing unlock, loads and stores to data[1] (A) cannot be reordered
against the store to lock[1] (B) due to TSO (all we care about is stores
reordered after loads).
On CPU 1 grabbing the 2nd lock, we care about loads/store from/to
data[1] (C) being reordered before load, test of lock[1] (C). Again,
thanks to TSO, loads/stores from/to data[1] (D) cannot be reordered
wrt (C).
So on x86-TSO we should be OK, but as you say, we need to be careful
to have the right barriers in place on other architectures.
>
> Your RFC design certainly works (in principle -- I haven't looked at
> the code in detail), but I can't shake the feeling that it's overkill
If we can find a simpler way to achieve the same result, I'm all ears :)
> and that it could be improved to avoid unnecessary aborts every time
> the lock holder is scheduled out.
The restart is only performed if preemption occurs when the thread is
trying to grab the lock. Once the thread has the lock (it's now the
lock holder), it will not be aborted even if it is migrated.
>
> This isn't a problem in your RFC design, but if we wanted to come up
> with tighter primitives, we'd have to be quite careful to document
> exactly what memory ordering guarantees they come with.
Indeed.
>
> It may be that all architectures for which you care about the
> performance boost already have efficient acquire and release
> operations. Certainly x86 does, and I don't know how fast the new ARM
> instructions are, but I imagine they're pretty good.
We may even need memory barriers around lock and unlock on ARM. :/ ARM
smp_store_release() and smp_load_acquire() currently implements those with
smp_mb().
>
> It's too bad that not all architectures have a single-instruction
> unlocked compare-and-exchange.
Based on my benchmarks, it's not clear that single-instruction
unlocked CAS is actually faster than doing the same with many
instructions.
Thanks,
Mathieu
>
> --Andy
>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH] percpu system call: fast userspace percpu critical sections
From: Andy Lutomirski @ 2015-05-26 21:18 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Andi Kleen, Borislav Petkov, H. Peter Anvin, Lai Jiangshan,
Ben Maurer, Paul E. McKenney, Ingo Molnar, Josh Triplett,
Andrew Morton, Michael Kerrisk, Linux API, Linux Kernel,
Paul Turner, Peter Zijlstra, Linus Torvalds, Steven Rostedt,
Andrew Hunter
In-Reply-To: <821493560.8531.1432674243321.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On Tue, May 26, 2015 at 2:04 PM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> ----- Original Message -----
>> On May 25, 2015 11:54 AM, "Andy Lutomirski" <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> [...]
>> I might be guilty of being too x86-centric here. On x86, as long as
>> the lock and unlock primitives are sufficiently atomic, everything
>> should be okay. On other architectures, though, a primitive that
>> gives lock, unlock, and abort of a per-cpu lock without checking that
>> you're still on the right cpu at unlock time may not be sufficient.
>> If the primitive is implemented purely with loads and stores, then
>> even if you take the lock, migrate, finish your work, and unlock
>> without anyone else contending from the lock (and hence don't abort),
>> the next thread to take the same lock will end up unsynchronized
>> unless there's appropriate memory ordering. For example, if taking
>> the lock were an acquire and unlocking were a release, we'd be fine.
>
> If we look at x86-TSO, where stores reordered after loads is pretty
> much the only thing we need to care about, we have:
>
> CPU 0 CPU 1
>
> load, test lock[1]
> store lock[1]
> loads/stores to data[1]
> [preempted, migrate to cpu 0]
> [run other thread]
> load, test lock[1] (loop)
> loads/stores to data[1] (A)
> store 0 to lock[1] (B)
> load, test lock[1] (C)
> store lock[1]
> loads/stores to data[1] (D)
>
> What we want to ensure is that load/stores to data[1] from CPU 0 and
> 1 don't get interleaved.
>
> On CPU 0 doing unlock, loads and stores to data[1] (A) cannot be reordered
> against the store to lock[1] (B) due to TSO (all we care about is stores
> reordered after loads).
>
> On CPU 1 grabbing the 2nd lock, we care about loads/store from/to
> data[1] (C) being reordered before load, test of lock[1] (C). Again,
> thanks to TSO, loads/stores from/to data[1] (D) cannot be reordered
> wrt (C).
>
> So on x86-TSO we should be OK, but as you say, we need to be careful
> to have the right barriers in place on other architectures.
>
>>
>> Your RFC design certainly works (in principle -- I haven't looked at
>> the code in detail), but I can't shake the feeling that it's overkill
>
> If we can find a simpler way to achieve the same result, I'm all ears :)
>
>> and that it could be improved to avoid unnecessary aborts every time
>> the lock holder is scheduled out.
>
> The restart is only performed if preemption occurs when the thread is
> trying to grab the lock. Once the thread has the lock (it's now the
> lock holder), it will not be aborted even if it is migrated.
Then maybe this does have the same acquire/release issue after all.
>
>>
>> This isn't a problem in your RFC design, but if we wanted to come up
>> with tighter primitives, we'd have to be quite careful to document
>> exactly what memory ordering guarantees they come with.
>
> Indeed.
>
>>
>> It may be that all architectures for which you care about the
>> performance boost already have efficient acquire and release
>> operations. Certainly x86 does, and I don't know how fast the new ARM
>> instructions are, but I imagine they're pretty good.
>
> We may even need memory barriers around lock and unlock on ARM. :/ ARM
> smp_store_release() and smp_load_acquire() currently implements those with
> smp_mb().
I would have expected it to use LDA/STL on new enough cpus:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0802a/CIHDEACA.html
>
>>
>> It's too bad that not all architectures have a single-instruction
>> unlocked compare-and-exchange.
>
> Based on my benchmarks, it's not clear that single-instruction
> unlocked CAS is actually faster than doing the same with many
> instructions.
True, but with a single instruction the user can't get preempted in the middle.
Looking at your code, it looks like percpu_user_sched_in has some
potentially nasty issues with page faults. Avoiding touching user
memory from the scheduler would be quite nice from an implementation
POV, and the x86-specific gs hack wins in that regard.
--Andy
^ permalink raw reply
* Re: [RFC PATCH] percpu system call: fast userspace percpu critical sections
From: Andi Kleen @ 2015-05-26 21:20 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Mathieu Desnoyers, H. Peter Anvin, Andi Kleen, Borislav Petkov,
Lai Jiangshan, Paul E. McKenney, Ben Maurer, Josh Triplett,
Ingo Molnar, Andrew Morton, Linux API, Michael Kerrisk,
Linux Kernel, Linus Torvalds, Peter Zijlstra, Paul Turner,
Steven Rostedt, Andrew Hunter
In-Reply-To: <CALCETrW3_Hv0jc3cpiwsHTinBqJzvab_EiPS8BVJhX-xe5D8qw@mail.gmail.com>
I haven't thought too much about it, but doing it in a vdso seems
reasonable. It would mean that the scheduler hot path would
need to know about its address though.
> > You appear to summarize the two things that are added to the kernel with
> > this RFC patch:
> > - faster getcpu(): 12.7 ns -> 0.3 ns
>
> Yeah, I figured that out the second time I read your email and re-read
> the original message, but I apparently forgot to fix up the email I
> was typing.
The high speed users seem to have all switched to using LSL directly,
which is even faster. Since there are already users it cannot be changed
anymore. Should probably just document this as an official ABI,
and provide some standard include file with inline code.
> IIRC some other OS's use gs as a userspace per-cpu pointer instead of
> a per-thread pointer. Would we want to consider enabling that?
> This may interact oddly with the WIP wrgsbase stuff. (Grr, Intel, why
> couldn't you have made wrgsbase reset the gs selector to zero? That
> would have been so much nicer.)
Applications really want the extra address registers. For example
OpenJDK could free a GPR. And it's needed for the TLS of user space
threads, which are becoming more and more popular. So there are
already more important users queued.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [RFC PATCH] percpu system call: fast userspace percpu critical sections
From: Andy Lutomirski @ 2015-05-26 21:26 UTC (permalink / raw)
To: Andi Kleen
Cc: Mathieu Desnoyers, H. Peter Anvin, Borislav Petkov, Lai Jiangshan,
Paul E. McKenney, Ben Maurer, Josh Triplett, Ingo Molnar,
Andrew Morton, Linux API, Michael Kerrisk, Linux Kernel,
Linus Torvalds, Peter Zijlstra, Paul Turner, Steven Rostedt,
Andrew Hunter
In-Reply-To: <20150526212041.GQ19417-1g7Xle2YJi4/4alezvVtWx2eb7JE58TQ@public.gmane.org>
On Tue, May 26, 2015 at 2:20 PM, Andi Kleen <andi-Vw/NltI1exuRpAAqCnN02g@public.gmane.org> wrote:
>
> I haven't thought too much about it, but doing it in a vdso seems
> reasonable. It would mean that the scheduler hot path would
> need to know about its address though.
>
>> > You appear to summarize the two things that are added to the kernel with
>> > this RFC patch:
>> > - faster getcpu(): 12.7 ns -> 0.3 ns
>>
>> Yeah, I figured that out the second time I read your email and re-read
>> the original message, but I apparently forgot to fix up the email I
>> was typing.
>
> The high speed users seem to have all switched to using LSL directly,
> which is even faster. Since there are already users it cannot be changed
> anymore. Should probably just document this as an official ABI,
> and provide some standard include file with inline code.
>
>> IIRC some other OS's use gs as a userspace per-cpu pointer instead of
>> a per-thread pointer. Would we want to consider enabling that?
>> This may interact oddly with the WIP wrgsbase stuff. (Grr, Intel, why
>> couldn't you have made wrgsbase reset the gs selector to zero? That
>> would have been so much nicer.)
>
> Applications really want the extra address registers. For example
> OpenJDK could free a GPR. And it's needed for the TLS of user space
> threads, which are becoming more and more popular. So there are
> already more important users queued.
This isn't necessarily the end of the world. They could turn it off
or hopefully repurpose or extend their FS usage instead of requiring
GS.
IOW, I think that ARCH_SET_GS should continue working, as should
wrgsbase. We could allow a program, strictly as an alternative, to do
ARCH_SET_GS_PERCPU or something. (If we do that, we should wire up
arch_prctl for x86_32 as well. We could further consider restricting
the new per-cpu mode to require a specific *nonzero* value for the
selector. Also, it's really annoying that we'll be forced to assign
some meaning to a non-zero selector with unexpected gsbase during
context switch.)
--Andy
^ permalink raw reply
* Re: [RFC PATCH] percpu system call: fast userspace percpu critical sections
From: Andy Lutomirski @ 2015-05-26 21:44 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Andi Kleen, Borislav Petkov, H. Peter Anvin, Lai Jiangshan,
Ben Maurer, Paul E. McKenney, Ingo Molnar, Josh Triplett,
Andrew Morton, Michael Kerrisk, Linux API, Linux Kernel,
Paul Turner, Peter Zijlstra, Linus Torvalds, Steven Rostedt,
Andrew Hunter
In-Reply-To: <CALCETrXXtu1BNqQopd5eCfa6wE9O+001ofhjjsXUe2f9K_hj8g@mail.gmail.com>
On Tue, May 26, 2015 at 2:18 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Tue, May 26, 2015 at 2:04 PM, Mathieu Desnoyers
>>
>>>
>>> It's too bad that not all architectures have a single-instruction
>>> unlocked compare-and-exchange.
>>
>> Based on my benchmarks, it's not clear that single-instruction
>> unlocked CAS is actually faster than doing the same with many
>> instructions.
>
> True, but with a single instruction the user can't get preempted in the middle.
>
> Looking at your code, it looks like percpu_user_sched_in has some
> potentially nasty issues with page faults. Avoiding touching user
> memory from the scheduler would be quite nice from an implementation
> POV, and the x86-specific gs hack wins in that regard.
ARM has "TLB lockdown entries" which could, I think, be used to
implement per-cpu or per-thread mappings. I'm actually rather
surprised that Linux doesn't already use a TLB lockdown entry for TLS.
(Hmm. Maybe it's because the interface to write the entries requires
actually touching the page. Maybe not -- the ARM docs, in general,
seem to be much less clear than the Intel and AMD docs.)
ARM doesn't seem to have any single-instruction compare-exchange or
similar instruction, though, so this might be all that useful. On the
other hand, ARM can probably do reasonably efficient per-cpu memory
allocation and such with a single ldrex/strex pair.
--Andy
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Casey Schaufler @ 2015-05-27 1:04 UTC (permalink / raw)
To: Lukasz Pawelczyk, David S. Miller, Eric W. Biederman,
Kirill A. Shutemov, Serge E. Hallyn, Al Viro, Alexey Dobriyan,
Andrew Morton, Andy Lutomirski, David Howells, Fabian Frederick,
Greg KH, James Morris, Jeff Layton, Jingoo Han, Joe Perches,
John Johansen, Jonathan Corbet, Kees Cook, Mauro Carvalho Chehab,
Miklos Szeredi, Oleg Nesterov, Paul Moore <paul@
Cc: Lukasz Pawelczyk, Casey Schaufler
In-Reply-To: <1432557162-19123-1-git-send-email-l.pawelczyk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On 5/25/2015 5:32 AM, Lukasz Pawelczyk wrote:
> Hello,
>
> Some time ago I sent a Smack namespace documentation and a preliminary
> LSM namespace for RFC. I've been suggested that there shouldn't be a
> separate LSM namespace and that it should live within user namespace.
> And this version does. This is a complete set of patches required for
> Smack namespace.
>
> This was designed with a collaboration of Smack maintainer Casey
> Schaufler.
>
> Smack namespace have been implemented using user namespace hooks added
> by one of the patches. To put some context to it I paste here a
> documentation on what Smack namespace wants to achieve.
>
> LSM hooks themselves are documented in the security.h file inside the
> patch.
>
> The patches are based on:
> https://github.com/cschaufler/smack-next/tree/smack-for-4.2-stacked
>
> The tree with them is avaiable here:
> https://github.com/Havner/smack-namespace/tree/smack-namespace-for-4.2-stacked-v2
>
> Changes from v1:
> - "kernel/exit.c: make sure current's nsproxy != NULL while checking
> caps" patch has been dropped
> - fixed the title of the user_ns operations patch
>
>
> ===================================================================
>
> --- What is a Smack namespace ---
>
> Smack namespace was developed to make it possible for Smack to work
> nicely with Linux containers where there is a full operating system
> with its own init inside the namespace. Such a system working with
> Smack expects to have at least partially working SMACK_MAC_ADMIN to be
> able to change labels of processes and files. This is required to be
> able to securely start applications under the control of Smack and
> manage their access rights.
>
> It was implemented using new LSM hooks added to the user namespace
> that were developed together with Smack namespace.
>
>
> --- Design ideas ---
>
> "Smack namespace" is rather "Smack labels namespace" as not the whole
> MAC is namespaced, only the labels. There is a great analogy between
> Smack labels namespace and the user namespace part that remaps UIDs.
>
> The idea is to create a map of labels for a namespace so the namespace
> is only allowed to use those labels. Smack rules are always the same
> as in the init namespace (limited only by what labels are mapped) and
> cannot be manipulated from the child namespace. The map is actually
> only for labels' names. The underlying structures for labels remain
> the same. The filesystem also stores the "unmapped" labels from the
> init namespace.
>
> Let's say we have those labels in the init namespace:
> label1
> label2
> label3
>
> and those rules:
> label1 label2 rwx
> label1 label3 rwx
> label2 label3 rwx
>
> We create a map for a namespace:
> label1 -> mapped1
> label2 -> mapped2
>
> This means that 'label3' is completely invisible in the namespace. As if
> it didn't exist. All the rules that include it are ignored.
>
> Effectively in the namespace we have only one rule:
> mapped1 mapped2 rwx
>
> Which in reality is:
> label1 label2 rwx
>
> All requests to access an object with a 'label3' will be denied. If it
> ever comes to a situation where 'label3' would have to be printed
> (e.g. reading an exec or mmap label from a file to which we have
> access) then huh sign '?' will be printed instead.
>
> All the operations in the namespace on the remaining labels will have
> to be performed using their mapped names. Things like changing own
> process's label, changing filesystem label. Labels will also be
> printed with their mapped names.
>
> You cannot import new labels in a namespace. Every operation that
> would do so in an init namespace will return an error in the child
> namespace. You cannot assign an unmapped or not existing label to an
> object. You can only operate on labels that have been explicitly
> mapped.
>
>
> --- Capabilities ---
>
> Enabling Smack related capabilities (CAP_MAC_ADMIN and
> CAP_MAC_OVERRIDE) is main goal of Smack namespace, so it can work
> properly in the container. And those capabilities do work to some
> extent. In several places where capabilities are checked compatibility
> with Smack namespace has been introduced. Capabilities are of course
> limited to operate only on mapped labels.
>
> CAP_MAC_OVERRIDE works fully, will allow you to ignore Smack access
> rules, but only between objects that have labels mapped. So in the
> example above having this CAP will allow e.g. label2 to write to
> label1, but will not allow any access to label3.
>
> With CAP_MAC_ADMIN the following operations has been allowed inside
> the namespace:
> - setting and removing xattr on files, including the security.* ones
> - setting process's own label (/proc/self/attr/current)
> - mounting in a privileged Smack mode, which means one can specify
> additional mount options like: smackfsdef, smackfsfloor etc.
>
> Again this is also allowed only on the mapped labels. Labels on the
> filesystem will be stored in unmapped form so they are preserved
> through reboots.
>
> Such a namespace construct allows e.g. systemd (with Smack support)
> working in a container to assign labels properly to daemons and other
> processes.
>
>
> --- Usage ---
>
> Smack namespace is written using LSM hooks inside user namespace. That
> means it's connected to it.
>
> To create a new Smack namespace you need to unshare() user namespace
> as usual. If that is all you do though, than there is no difference to
> what is now. To activate the Smack namespace you need to fill the
> labels' map. It is in a file /proc/$PID/smack_map.
>
> By default the map is empty and Smack namespaces are inactive (labels
> are taken directly from a parent namespace). It also means that the
> Smack capabilities will be inactive. After you fill the map it starts
> to take effect in the namespace and Smack capabilities (only on mapped
> labels) start to work.
>
> Due to the way Smack works only CAP_MAC_ADMIN from the parent
> namespace (init_user_ns for now, see the "Current limitations" below)
> is allowed to fill the map. That means that an unprivileged user is
> still allowed to create the user namespace but it will not be able to
> fill the labels' map (activate Smack namespace). An administrator
> intervention is required.
>
> The attr_map write format is:
> unmapped_label mapped_label
>
> When reading the file it shows an active map for a namespace the
> process in question is in in the format:
> unmapped_label -> mapped_label
>
> If the smack_map file is empty it means the namespace is not mapped
> and Smack namespace is inactive (no mappings, MAC related capabilities
> behave as they did before, meaning they are active only in
> init_user_ns). For init_user_ns the map will always be empty.
>
> Writing to the map file is not disabled after the first write as it is
> in uid_map. For Smack we have no means to map ranges of labels, hence
> it can really be advantageous to be able to expand the map later
> on. But you can only add to the map. You cannot remove already mapped
> labels. You cannot change the already existing mappings. Also mappings
> has to be 1-1. All requests to create a map where either the unmapped
> or the mapped label already exists in the map will be denied.
>
> setns() with Smack namespace active has an additional check that the
> label of a process that is calling setns() has to be already mapped in
> the target Smack namespace for the call to succeed.
>
>
> --- Special labels ---
>
> Smack is using some special labels that have built-in rules. Things
> like floor '_', dash '^', star '*', etc. Those labels are not
> automatically mapped to the namespace. Moreover, you can choose to map
> a different label from the init namespace to behave e.g. like floor
> inside the namespace.
>
> Let's say we have no rules and those labels in the init namespace:
> _
> floor_to_be
> label
>
> Both 'label' and 'floor_to_be' can read objects with '_'. But they
> have no access rights to each other.
>
> Now let's create a map like this:
> _ ordinary_label
> floor_to_be _
> label mapped
>
> Right now label 'mapped' can read label '_' which means that
> effectively inside this namespace label 'label' has gained read access
> to the 'floor_to_be'. The label 'ordinary_label' is exactly it, an
> ordinary label that the built-in rules no longer apply to inside the
> namespace.
>
> To sum up, special labels in the namespace behave the same as in the
> init namespace. Not the original special labels though, but the ones
> we map to specials. This is the only case where a namespace can have
> access rights the init namespace does not have (like the 'label' to
> 'floor_to_be' in the example above).
>
> Of course mappings like these are perfectly legal:
> _ _
> * *
> ^ ^
>
>
> --- Current limitations ---
>
> The Smack namespace is not hierarchical yet. It is currently not
> possible to fill a smack_map of a nested user namespace (you can still
> create nested user namespace, it will just inherit its parent's map
> and won't have active Smack capabilities). When hierarchy will be
> implemented the process creating another namespace will be allowed to
> map only labels that it has permission to itself (those that it has in
> its own map).
>
> Special files inside the virtual smackfs needs to be reviewed whether
> it's beneficial to have some of their functionality namespaced as well
> (e.g. onlycap, syslog. ambient, etc). This would increase
> CAP_MAC_ADMIN privileges inside the namespace.
>
>
> Lukasz Pawelczyk (7):
> user_ns: 3 new hooks for user namespace operations
> smack: extend capability functions and fix 2 checks
> smack: abstraction layer for 2 common Smack operations
> smack: misc cleanups in preparation for a namespace patch
> smack: namespace groundwork
> smack: namespace implementation
> smack: documentation for the Smack namespace
>
> Documentation/security/00-INDEX | 2 +
> Documentation/security/Smack-namespace.txt | 231 +++++++++++++
> MAINTAINERS | 1 +
> fs/proc/base.c | 57 +++
> include/linux/lsm_hooks.h | 28 ++
> include/linux/security.h | 23 ++
> include/linux/user_namespace.h | 9 +
> kernel/user.c | 3 +
> kernel/user_namespace.c | 18 +
> security/security.c | 28 ++
> security/smack/Kconfig | 12 +
> security/smack/Makefile | 1 +
> security/smack/smack.h | 187 +++++++++-
> security/smack/smack_access.c | 191 ++++++++--
> security/smack/smack_lsm.c | 536 ++++++++++++++++++++---------
> security/smack/smack_ns.c | 471 +++++++++++++++++++++++++
> security/smack/smackfs.c | 154 +++++----
> 17 files changed, 1702 insertions(+), 250 deletions(-)
> create mode 100644 Documentation/security/Smack-namespace.txt
> create mode 100644 security/smack/smack_ns.c
>
I have reviewed these patches (again!) and don't have
any issues. I see that Mr. Smalley has some insightful
suggestions, and once those are addressed I think that
we should be lined up to sell it to the infrastructure.
^ permalink raw reply
* Re: [RFC v2 1/4] fs: Add generic file system event notifications
From: Greg KH @ 2015-05-27 2:34 UTC (permalink / raw)
To: Beata Michalska
Cc: Jan Kara, linux-kernel, linux-fsdevel, linux-api, tytso,
adilger.kernel, hughd, lczerner, hch, linux-ext4, linux-mm,
kyungmin.park, kmpark
In-Reply-To: <5564A1D4.4040309@samsung.com>
On Tue, May 26, 2015 at 06:39:48PM +0200, Beata Michalska wrote:
> Hi,
>
> Things has gone a bit quiet thread wise ...
> As I believe I've managed to snap back to reality, I was hoping we could continue with this?
> I'm not sure if we've got everything cleared up or ... have we reached a dead end?
> Please let me know if we can move to the next stage? Or, if there are any showstoppers?
Please resend if you think it's ready and you have addressed the issues
raised so far.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Eric W. Biederman @ 2015-05-27 3:13 UTC (permalink / raw)
To: Lukasz Pawelczyk
Cc: Lukasz Pawelczyk, linux-doc-u79uwXL29TY76Z2rM5mHXA, Tetsuo Handa,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells, Paul Moore,
Jonathan Corbet, Jingoo Han, Jeff Layton, Stephen Smalley,
Alexey Dobriyan, Kees Cook, Mauro Carvalho Chehab,
Fabian Frederick, Casey Schaufler, Al Viro, James Morris,
Kirill A. Shutemov, John Johansen, Rafal Krypa, Greg KH,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Miklos Szeredi, Oleg
In-Reply-To: <1432557162-19123-1-git-send-email-l.pawelczyk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Lukasz Pawelczyk <l.pawelczyk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> writes:
> Hello,
>
> Some time ago I sent a Smack namespace documentation and a preliminary
> LSM namespace for RFC. I've been suggested that there shouldn't be a
> separate LSM namespace and that it should live within user namespace.
> And this version does. This is a complete set of patches required for
> Smack namespace.
>
> This was designed with a collaboration of Smack maintainer Casey
> Schaufler.
>
> Smack namespace have been implemented using user namespace hooks added
> by one of the patches. To put some context to it I paste here a
> documentation on what Smack namespace wants to achieve.
>
> LSM hooks themselves are documented in the security.h file inside the
> patch.
>
> The patches are based on:
> https://github.com/cschaufler/smack-next/tree/smack-for-4.2-stacked
>
> The tree with them is avaiable here:
> https://github.com/Havner/smack-namespace/tree/smack-namespace-for-4.2-stacked-v2
>
> Changes from v1:
> - "kernel/exit.c: make sure current's nsproxy != NULL while checking
> caps" patch has been dropped
> - fixed the title of the user_ns operations patch
A have not completed a review I don't understand smack well enough to
answer some questions but I don't think I like the approach this patch
takes to get things done.
I am flattered that you are using a mapping as I did with uid map and
gid map. Unfortunately I do not believe your mapping achieves what my
mapping of uids and gids achieved.
A technical goal is to give people the tools so that a sysadmin can set
up a system, can grant people subids and subgids, and then the user can
proceed to do what they need to do. In particular there should be
little to no need to keep pestering the system administrator for more
identifiers.
The flip side of that was that the mapping would ensure all of the
existing permissions checks would work as expected, and the checks in
the kernel could be converted without much trouble.
Ranges of ids were choosen because they allow for a lot of possible ways
of using uids and gids in containers, are comparitively easy to
administer, are very fast to use, and don't need large data structures.
With a discreet mapping of labels I have the premonition that we now
have a large data structure that, is not as flexible in to use,
is comparatively slow and appears to require an interaction with the
system administrator for every label you use in a container.
As part of that there is added a void *security pointer in the user
namespace to apparently hang off anything anyone would like to use.
Connected to that are hooks that have failure codes (presumably memory
allocation failures), but the semantics are not clear. My gut feel is
that I would rather extend struct user_namespace to hold the smack label
mapping table and remove all of the error codes because they would then
be unnecessary.
I am also concerned that several of the operations assume that setns
and the like are normally privileged operations and so require the
ability to perform other privileged operations. Given that in the right
circumstances setns is not privileged that seems like a semantics
mismatch.
Or in short my gut feel says the semantics of this change are close to
something that would be very useful, but the details make this patchset
far less useful, usable and comprehensible than it should be.
Eric
> ===================================================================
>
> --- What is a Smack namespace ---
>
> Smack namespace was developed to make it possible for Smack to work
> nicely with Linux containers where there is a full operating system
> with its own init inside the namespace. Such a system working with
> Smack expects to have at least partially working SMACK_MAC_ADMIN to be
> able to change labels of processes and files. This is required to be
> able to securely start applications under the control of Smack and
> manage their access rights.
>
> It was implemented using new LSM hooks added to the user namespace
> that were developed together with Smack namespace.
>
>
> --- Design ideas ---
>
> "Smack namespace" is rather "Smack labels namespace" as not the whole
> MAC is namespaced, only the labels. There is a great analogy between
> Smack labels namespace and the user namespace part that remaps UIDs.
>
> The idea is to create a map of labels for a namespace so the namespace
> is only allowed to use those labels. Smack rules are always the same
> as in the init namespace (limited only by what labels are mapped) and
> cannot be manipulated from the child namespace. The map is actually
> only for labels' names. The underlying structures for labels remain
> the same. The filesystem also stores the "unmapped" labels from the
> init namespace.
>
> Let's say we have those labels in the init namespace:
> label1
> label2
> label3
>
> and those rules:
> label1 label2 rwx
> label1 label3 rwx
> label2 label3 rwx
>
> We create a map for a namespace:
> label1 -> mapped1
> label2 -> mapped2
>
> This means that 'label3' is completely invisible in the namespace. As if
> it didn't exist. All the rules that include it are ignored.
>
> Effectively in the namespace we have only one rule:
> mapped1 mapped2 rwx
>
> Which in reality is:
> label1 label2 rwx
>
> All requests to access an object with a 'label3' will be denied. If it
> ever comes to a situation where 'label3' would have to be printed
> (e.g. reading an exec or mmap label from a file to which we have
> access) then huh sign '?' will be printed instead.
>
> All the operations in the namespace on the remaining labels will have
> to be performed using their mapped names. Things like changing own
> process's label, changing filesystem label. Labels will also be
> printed with their mapped names.
>
> You cannot import new labels in a namespace. Every operation that
> would do so in an init namespace will return an error in the child
> namespace. You cannot assign an unmapped or not existing label to an
> object. You can only operate on labels that have been explicitly
> mapped.
>
>
> --- Capabilities ---
>
> Enabling Smack related capabilities (CAP_MAC_ADMIN and
> CAP_MAC_OVERRIDE) is main goal of Smack namespace, so it can work
> properly in the container. And those capabilities do work to some
> extent. In several places where capabilities are checked compatibility
> with Smack namespace has been introduced. Capabilities are of course
> limited to operate only on mapped labels.
>
> CAP_MAC_OVERRIDE works fully, will allow you to ignore Smack access
> rules, but only between objects that have labels mapped. So in the
> example above having this CAP will allow e.g. label2 to write to
> label1, but will not allow any access to label3.
>
> With CAP_MAC_ADMIN the following operations has been allowed inside
> the namespace:
> - setting and removing xattr on files, including the security.* ones
> - setting process's own label (/proc/self/attr/current)
> - mounting in a privileged Smack mode, which means one can specify
> additional mount options like: smackfsdef, smackfsfloor etc.
>
> Again this is also allowed only on the mapped labels. Labels on the
> filesystem will be stored in unmapped form so they are preserved
> through reboots.
>
> Such a namespace construct allows e.g. systemd (with Smack support)
> working in a container to assign labels properly to daemons and other
> processes.
>
>
> --- Usage ---
>
> Smack namespace is written using LSM hooks inside user namespace. That
> means it's connected to it.
>
> To create a new Smack namespace you need to unshare() user namespace
> as usual. If that is all you do though, than there is no difference to
> what is now. To activate the Smack namespace you need to fill the
> labels' map. It is in a file /proc/$PID/smack_map.
>
> By default the map is empty and Smack namespaces are inactive (labels
> are taken directly from a parent namespace). It also means that the
> Smack capabilities will be inactive. After you fill the map it starts
> to take effect in the namespace and Smack capabilities (only on mapped
> labels) start to work.
>
> Due to the way Smack works only CAP_MAC_ADMIN from the parent
> namespace (init_user_ns for now, see the "Current limitations" below)
> is allowed to fill the map. That means that an unprivileged user is
> still allowed to create the user namespace but it will not be able to
> fill the labels' map (activate Smack namespace). An administrator
> intervention is required.
>
> The attr_map write format is:
> unmapped_label mapped_label
>
> When reading the file it shows an active map for a namespace the
> process in question is in in the format:
> unmapped_label -> mapped_label
>
> If the smack_map file is empty it means the namespace is not mapped
> and Smack namespace is inactive (no mappings, MAC related capabilities
> behave as they did before, meaning they are active only in
> init_user_ns). For init_user_ns the map will always be empty.
>
> Writing to the map file is not disabled after the first write as it is
> in uid_map. For Smack we have no means to map ranges of labels, hence
> it can really be advantageous to be able to expand the map later
> on. But you can only add to the map. You cannot remove already mapped
> labels. You cannot change the already existing mappings. Also mappings
> has to be 1-1. All requests to create a map where either the unmapped
> or the mapped label already exists in the map will be denied.
>
> setns() with Smack namespace active has an additional check that the
> label of a process that is calling setns() has to be already mapped in
> the target Smack namespace for the call to succeed.
>
> --- Special labels ---
>
> Smack is using some special labels that have built-in rules. Things
> like floor '_', dash '^', star '*', etc. Those labels are not
> automatically mapped to the namespace. Moreover, you can choose to map
> a different label from the init namespace to behave e.g. like floor
> inside the namespace.
>
> Let's say we have no rules and those labels in the init namespace:
> _
> floor_to_be
> label
>
> Both 'label' and 'floor_to_be' can read objects with '_'. But they
> have no access rights to each other.
>
> Now let's create a map like this:
> _ ordinary_label
> floor_to_be _
> label mapped
>
> Right now label 'mapped' can read label '_' which means that
> effectively inside this namespace label 'label' has gained read access
> to the 'floor_to_be'. The label 'ordinary_label' is exactly it, an
> ordinary label that the built-in rules no longer apply to inside the
> namespace.
>
> To sum up, special labels in the namespace behave the same as in the
> init namespace. Not the original special labels though, but the ones
> we map to specials. This is the only case where a namespace can have
> access rights the init namespace does not have (like the 'label' to
> 'floor_to_be' in the example above).
>
> Of course mappings like these are perfectly legal:
> _ _
> * *
> ^ ^
>
>
> --- Current limitations ---
>
> The Smack namespace is not hierarchical yet. It is currently not
> possible to fill a smack_map of a nested user namespace (you can still
> create nested user namespace, it will just inherit its parent's map
> and won't have active Smack capabilities). When hierarchy will be
> implemented the process creating another namespace will be allowed to
> map only labels that it has permission to itself (those that it has in
> its own map).
> Special files inside the virtual smackfs needs to be reviewed whether
> it's beneficial to have some of their functionality namespaced as well
> (e.g. onlycap, syslog. ambient, etc). This would increase
> CAP_MAC_ADMIN privileges inside the namespace.
>
>
> Lukasz Pawelczyk (7):
> user_ns: 3 new hooks for user namespace operations
> smack: extend capability functions and fix 2 checks
> smack: abstraction layer for 2 common Smack operations
> smack: misc cleanups in preparation for a namespace patch
> smack: namespace groundwork
> smack: namespace implementation
> smack: documentation for the Smack namespace
>
> Documentation/security/00-INDEX | 2 +
> Documentation/security/Smack-namespace.txt | 231 +++++++++++++
> MAINTAINERS | 1 +
> fs/proc/base.c | 57 +++
> include/linux/lsm_hooks.h | 28 ++
> include/linux/security.h | 23 ++
> include/linux/user_namespace.h | 9 +
> kernel/user.c | 3 +
> kernel/user_namespace.c | 18 +
> security/security.c | 28 ++
> security/smack/Kconfig | 12 +
> security/smack/Makefile | 1 +
> security/smack/smack.h | 187 +++++++++-
> security/smack/smack_access.c | 191 ++++++++--
> security/smack/smack_lsm.c | 536 ++++++++++++++++++++---------
> security/smack/smack_ns.c | 471 +++++++++++++++++++++++++
> security/smack/smackfs.c | 154 +++++----
> 17 files changed, 1702 insertions(+), 250 deletions(-)
> create mode 100644 Documentation/security/Smack-namespace.txt
> create mode 100644 security/smack/smack_ns.c
^ permalink raw reply
* Re: [PATCH 2/2] fbcon: use the cursor blink interval provided by vt
From: Andrey Wagin @ 2015-05-27 5:57 UTC (permalink / raw)
To: Scot Doyle
Cc: Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
Greg Kroah-Hartman, Jiri Slaby, Michael Kerrisk, Pavel Machek,
Geert Uytterhoeven, LKML, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-man-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.LNX.2.11.1502271914040.4248@localhost>
2015-02-27 22:15 GMT+03:00 Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>:
> vt now provides a cursor blink interval via vc_data. Use this
> interval instead of the currently hardcoded 200 msecs. Store it in
> fbcon_ops to avoid locking the console in cursor_timer_handler().
I regularly execute criu tests on linux-next. For this, I use virtual
machine from the digitalocean clould. The current version of
linux-next hangs after a few seconds. I use git bisect to find the
commit where the problem is appeaed. And it looks like the problem is
in this patch.
When the kernel hangs, it doesn't report anything on the screen and
there is nothing suspicious in logs after reboot.
I will try to reproduce the problem in my local enviroment to get more
information.
There is my config file:
https://github.com/avagin/criu-jenkins-digitalocean/blob/d95d9e30a7da8755c47b290630bac7ee1fe7132d/jenkins-scripts/config
Thanks,
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3] fs, proc: introduce CONFIG_PROC_CHILDREN
From: Cyrill Gorcunov @ 2015-05-27 7:29 UTC (permalink / raw)
To: Iago López Galeiras
Cc: Oleg Nesterov, Kees Cook, Pavel Emelyanov, Serge Hallyn,
KAMEZAWA Hiroyuki, linux-api-u79uwXL29TY76Z2rM5mHXA,
Alexander Viro, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
Andrew Morton, Andy Lutomirski, Djalal Harouni, Alban Crequy
In-Reply-To: <1432653162-20973-1-git-send-email-iago-973cpzSjLbNWk0Htik3J/w@public.gmane.org>
On Tue, May 26, 2015 at 05:12:42PM +0200, Iago López Galeiras wrote:
...
> Alban tested that /proc/<pid>/task/<tid>/children is present when the
> kernel is configured with CONFIG_PROC_CHILDREN=y but without
> CONFIG_CHECKPOINT_RESTORE
>
> v3: change depend -> select
> v2: introduce CONFIG_PROC_CHILDREN http://marc.info/?l=linux-fsdevel&m=143229890217280&w=2
> v1: http://marc.info/?l=linux-api&m=143220431121869&w=2
>
> Signed-off-by: Iago López Galeiras <iago-973cpzSjLbNWk0Htik3J/w@public.gmane.org>
> Cc: Alban Crequy <alban-973cpzSjLbNWk0Htik3J/w@public.gmane.org>
> Tested-by: Alban Crequy <alban-973cpzSjLbNWk0Htik3J/w@public.gmane.org>
Looks ok to me, thanks.
Reviewed-by: Cyrill Gorcunov <gorcunov-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
^ permalink raw reply
* Re: [PATCH 2/2] fbcon: use the cursor blink interval provided by vt
From: Scot Doyle @ 2015-05-27 7:52 UTC (permalink / raw)
To: Andrey Wagin
Cc: Greg Kroah-Hartman, Tomi Valkeinen,
Jean-Christophe Plagniol-Villard, Jiri Slaby, Michael Kerrisk,
Pavel Machek, Geert Uytterhoeven, LKML,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-man-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CANaxB-zX4GkExeCzwgYZVDVZE47DHOCnRiYe86difk4fRMK=9A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, 27 May 2015, Andrey Wagin wrote:
...
> I regularly execute criu tests on linux-next. For this, I use virtual
> machine from the digitalocean clould. The current version of
> linux-next hangs after a few seconds. I use git bisect to find the
> commit where the problem is appeaed. And it looks like the problem is
> in this patch.
...
> Thanks,
> Andrew
Perhaps this pending patch will help:
http://marc.info/?l=linux-kernel&m=143219509918969&w=2
Thanks,
Scot
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Lukasz Pawelczyk @ 2015-05-27 9:29 UTC (permalink / raw)
To: Casey Schaufler
Cc: David S. Miller, Eric W. Biederman, Kirill A. Shutemov,
Serge E. Hallyn, Al Viro, Alexey Dobriyan, Andrew Morton,
Andy Lutomirski, David Howells, Fabian Frederick, Greg KH,
James Morris, Jeff Layton, Jingoo Han, Joe Perches, John Johansen,
Jonathan Corbet, Kees Cook, Mauro Carvalho Chehab, Miklos Szeredi,
Oleg Nesterov, Paul Moore, Stephen Smalley
In-Reply-To: <55651823.10304@schaufler-ca.com>
On wto, 2015-05-26 at 18:04 -0700, Casey Schaufler wrote:
> On 5/25/2015 5:32 AM, Lukasz Pawelczyk wrote:
> > Hello,
> >
> > Some time ago I sent a Smack namespace documentation and a preliminary
> > LSM namespace for RFC. I've been suggested that there shouldn't be a
> > separate LSM namespace and that it should live within user namespace.
> > And this version does. This is a complete set of patches required for
> > Smack namespace.
> >
> > This was designed with a collaboration of Smack maintainer Casey
> > Schaufler.
> >
>
> I have reviewed these patches (again!) and don't have
> any issues. I see that Mr. Smalley has some insightful
> suggestions, and once those are addressed I think that
> we should be lined up to sell it to the infrastructure.
Thank you.
Can I add
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
to those 7 the patches?
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Lukasz Pawelczyk @ 2015-05-27 9:36 UTC (permalink / raw)
To: Stephen Smalley
Cc: David S. Miller, Eric W. Biederman, Kirill A. Shutemov,
Serge E. Hallyn, Al Viro, Alexey Dobriyan, Andrew Morton,
Andy Lutomirski, Casey Schaufler, David Howells, Fabian Frederick,
Greg KH, James Morris, Jeff Layton, Jingoo Han, Joe Perches,
John Johansen, Jonathan Corbet, Kees Cook, Mauro Carvalho Chehab,
Miklos Szeredi, Oleg Nesterov, Paul Moore
In-Reply-To: <556484BD.2060004@tycho.nsa.gov>
On wto, 2015-05-26 at 10:35 -0400, Stephen Smalley wrote:
> On 05/25/2015 08:32 AM, Lukasz Pawelczyk wrote:
> > --- Usage ---
> >
> > Smack namespace is written using LSM hooks inside user namespace. That
> > means it's connected to it.
> >
> > To create a new Smack namespace you need to unshare() user namespace
> > as usual. If that is all you do though, than there is no difference to
> > what is now. To activate the Smack namespace you need to fill the
> > labels' map. It is in a file /proc/$PID/smack_map.
>
> This should be /proc/$PID/attr/label_map or similar, modeled after the
> existing /proc/$PID/attr/current and similar nodes. Then it isn't
> module-specific and can be reused for other modules.
To make this generic I'll have to introduce new LSH hooks to handle this
file (much like /proc/$PID/attr/current).
I take this is what you had in mind.
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Lukasz Pawelczyk @ 2015-05-27 10:13 UTC (permalink / raw)
To: Eric W. Biederman, Casey Schaufler
Cc: David S. Miller, Kirill A. Shutemov, Serge E. Hallyn, Al Viro,
Alexey Dobriyan, Andrew Morton, Andy Lutomirski, David Howells,
Fabian Frederick, Greg KH, James Morris, Jeff Layton, Jingoo Han,
Joe Perches, John Johansen, Jonathan Corbet, Kees Cook,
Mauro Carvalho Chehab, Miklos Szeredi, Oleg Nesterov, Paul Moore,
Stephen Smalley, Tetsuo Handa
In-Reply-To: <87h9qyww4h.fsf@x220.int.ebiederm.org>
On wto, 2015-05-26 at 22:13 -0500, Eric W. Biederman wrote:
> Lukasz Pawelczyk <l.pawelczyk@samsung.com> writes:
>
> > Hello,
> >
> > Some time ago I sent a Smack namespace documentation and a preliminary
> > LSM namespace for RFC. I've been suggested that there shouldn't be a
> > separate LSM namespace and that it should live within user namespace.
> > And this version does. This is a complete set of patches required for
> > Smack namespace.
> >
> > This was designed with a collaboration of Smack maintainer Casey
> > Schaufler.
> >
> > Smack namespace have been implemented using user namespace hooks added
> > by one of the patches. To put some context to it I paste here a
> > documentation on what Smack namespace wants to achieve.
> >
> > LSM hooks themselves are documented in the security.h file inside the
> > patch.
> >
> > The patches are based on:
> > https://github.com/cschaufler/smack-next/tree/smack-for-4.2-stacked
> >
> > The tree with them is avaiable here:
> > https://github.com/Havner/smack-namespace/tree/smack-namespace-for-4.2-stacked-v2
> >
> > Changes from v1:
> > - "kernel/exit.c: make sure current's nsproxy != NULL while checking
> > caps" patch has been dropped
> > - fixed the title of the user_ns operations patch
>
> A have not completed a review I don't understand smack well enough to
> answer some questions but I don't think I like the approach this patch
> takes to get things done.
>
> I am flattered that you are using a mapping as I did with uid map and
> gid map. Unfortunately I do not believe your mapping achieves what my
> mapping of uids and gids achieved.
>
> A technical goal is to give people the tools so that a sysadmin can set
> up a system, can grant people subids and subgids, and then the user can
> proceed to do what they need to do.
This is exactly the case with Smack namespace.
> In particular there should be
> little to no need to keep pestering the system administrator for more
> identifiers.
This all depends on the use case. When you create a new namespace for a
particular purpose you could know what labels will be required there and
map them upfront. Adding new mappings is for a use case that a container
might have new software installed that requires new labels for it. You
don't create/import new labels on a normal basis all the time.
> The flip side of that was that the mapping would ensure all of the
> existing permissions checks would work as expected, and the checks in
> the kernel could be converted without much trouble.
Again, this is exactly the case with Smack namespace. There are
additional checks in place of course, but the rules are clear about
how that works.
> Ranges of ids were choosen because they allow for a lot of possible ways
> of using uids and gids in containers, are comparitively easy to
> administer, are very fast to use, and don't need large data structures.
This is Smack specific. The same way we could compare Smack with DAC
(even without talking about namespaces). Every new UID is just it, a
number. Every new label requires allocation and adds to the list.
I had an idea about mapping ranges of labels for a container by using
prefixes but this was dropped by Casey. Labels (with an exception of the
built-in ones) should not have any special meaning. This is Smack
design.
About performance, as it was originally with Smack. The list of labels
was a simple list. When the performance was not enough hashing was
added. I'd take the same approach here. If some use cases will require
so many mappings that this becomes a problem we will deal with it. I
don't want to complicate the patches now and I this was Casey's opinion
as well.
> With a discreet mapping of labels I have the premonition that we now
> have a large data structure that, is not as flexible in to use,
> is comparatively slow and appears to require an interaction with the
> system administrator for every label you use in a container.
Again, DAC vs Smack philosophy. The same way a new label appears in init
namespace (e.g. with a file/inode). Without a new rule you won't have an
access to it. This requires administrator intervention as well. This is
MAC after all.
> As part of that there is added a void *security pointer in the user
> namespace to apparently hang off anything anyone would like to use.
> Connected to that are hooks that have failure codes (presumably memory
> allocation failures), but the semantics are not clear. My gut feel is
> that I would rather extend struct user_namespace to hold the smack label
> mapping table and remove all of the error codes because they would then
> be unnecessary.
How is this different then filling a void *security pointer for other
kernel objects (tasks, indodes, ipc, etc). The allocations can fail as
well. I don't see how the semantics are different here.
So you would have me a Smack generic pointer in user_namespace without
any LSM abstraction? How does that cope with LSM philosophy and recently
introduced (initial) LSM stacking?
> I am also concerned that several of the operations assume that setns
> and the like are normally privileged operations and so require the
> ability to perform other privileged operations. Given that in the right
> circumstances setns is not privileged that seems like a semantics
> mismatch.
Sorry, I don't exactly know what you mean. setns remains unprivileged
operation here. The only limit is to refuse setns when adding a process
to a container would break Smack namespace rules. In theory this
limitation could be removed (I think), because a process with an
unmapped label would not be able to do anything in the namespace. This
is mostly for convenience.
> Or in short my gut feel says the semantics of this change are close to
> something that would be very useful, but the details make this patchset
> far less useful, usable and comprehensible than it should be.
This isn't much to go on for me unfortunately.
Anyway, thank you for your insight, and taking the time.
>
> Eric
>
>
> > ===================================================================
> >
> > --- What is a Smack namespace ---
> >
> > Smack namespace was developed to make it possible for Smack to work
> > nicely with Linux containers where there is a full operating system
> > with its own init inside the namespace. Such a system working with
> > Smack expects to have at least partially working SMACK_MAC_ADMIN to be
> > able to change labels of processes and files. This is required to be
> > able to securely start applications under the control of Smack and
> > manage their access rights.
> >
> > It was implemented using new LSM hooks added to the user namespace
> > that were developed together with Smack namespace.
> >
> >
> > --- Design ideas ---
> >
> > "Smack namespace" is rather "Smack labels namespace" as not the whole
> > MAC is namespaced, only the labels. There is a great analogy between
> > Smack labels namespace and the user namespace part that remaps UIDs.
> >
> > The idea is to create a map of labels for a namespace so the namespace
> > is only allowed to use those labels. Smack rules are always the same
> > as in the init namespace (limited only by what labels are mapped) and
> > cannot be manipulated from the child namespace. The map is actually
> > only for labels' names. The underlying structures for labels remain
> > the same. The filesystem also stores the "unmapped" labels from the
> > init namespace.
> >
> > Let's say we have those labels in the init namespace:
> > label1
> > label2
> > label3
> >
> > and those rules:
> > label1 label2 rwx
> > label1 label3 rwx
> > label2 label3 rwx
> >
> > We create a map for a namespace:
> > label1 -> mapped1
> > label2 -> mapped2
> >
> > This means that 'label3' is completely invisible in the namespace. As if
> > it didn't exist. All the rules that include it are ignored.
> >
> > Effectively in the namespace we have only one rule:
> > mapped1 mapped2 rwx
> >
> > Which in reality is:
> > label1 label2 rwx
> >
> > All requests to access an object with a 'label3' will be denied. If it
> > ever comes to a situation where 'label3' would have to be printed
> > (e.g. reading an exec or mmap label from a file to which we have
> > access) then huh sign '?' will be printed instead.
> >
> > All the operations in the namespace on the remaining labels will have
> > to be performed using their mapped names. Things like changing own
> > process's label, changing filesystem label. Labels will also be
> > printed with their mapped names.
> >
> > You cannot import new labels in a namespace. Every operation that
> > would do so in an init namespace will return an error in the child
> > namespace. You cannot assign an unmapped or not existing label to an
> > object. You can only operate on labels that have been explicitly
> > mapped.
> >
> >
> > --- Capabilities ---
> >
> > Enabling Smack related capabilities (CAP_MAC_ADMIN and
> > CAP_MAC_OVERRIDE) is main goal of Smack namespace, so it can work
> > properly in the container. And those capabilities do work to some
> > extent. In several places where capabilities are checked compatibility
> > with Smack namespace has been introduced. Capabilities are of course
> > limited to operate only on mapped labels.
> >
> > CAP_MAC_OVERRIDE works fully, will allow you to ignore Smack access
> > rules, but only between objects that have labels mapped. So in the
> > example above having this CAP will allow e.g. label2 to write to
> > label1, but will not allow any access to label3.
> >
> > With CAP_MAC_ADMIN the following operations has been allowed inside
> > the namespace:
> > - setting and removing xattr on files, including the security.* ones
> > - setting process's own label (/proc/self/attr/current)
> > - mounting in a privileged Smack mode, which means one can specify
> > additional mount options like: smackfsdef, smackfsfloor etc.
> >
> > Again this is also allowed only on the mapped labels. Labels on the
> > filesystem will be stored in unmapped form so they are preserved
> > through reboots.
> >
> > Such a namespace construct allows e.g. systemd (with Smack support)
> > working in a container to assign labels properly to daemons and other
> > processes.
> >
> >
> > --- Usage ---
> >
> > Smack namespace is written using LSM hooks inside user namespace. That
> > means it's connected to it.
> >
> > To create a new Smack namespace you need to unshare() user namespace
> > as usual. If that is all you do though, than there is no difference to
> > what is now. To activate the Smack namespace you need to fill the
> > labels' map. It is in a file /proc/$PID/smack_map.
> >
> > By default the map is empty and Smack namespaces are inactive (labels
> > are taken directly from a parent namespace). It also means that the
> > Smack capabilities will be inactive. After you fill the map it starts
> > to take effect in the namespace and Smack capabilities (only on mapped
> > labels) start to work.
> >
> > Due to the way Smack works only CAP_MAC_ADMIN from the parent
> > namespace (init_user_ns for now, see the "Current limitations" below)
> > is allowed to fill the map. That means that an unprivileged user is
> > still allowed to create the user namespace but it will not be able to
> > fill the labels' map (activate Smack namespace). An administrator
> > intervention is required.
> >
> > The attr_map write format is:
> > unmapped_label mapped_label
> >
> > When reading the file it shows an active map for a namespace the
> > process in question is in in the format:
> > unmapped_label -> mapped_label
> >
> > If the smack_map file is empty it means the namespace is not mapped
> > and Smack namespace is inactive (no mappings, MAC related capabilities
> > behave as they did before, meaning they are active only in
> > init_user_ns). For init_user_ns the map will always be empty.
> >
> > Writing to the map file is not disabled after the first write as it is
> > in uid_map. For Smack we have no means to map ranges of labels, hence
> > it can really be advantageous to be able to expand the map later
> > on. But you can only add to the map. You cannot remove already mapped
> > labels. You cannot change the already existing mappings. Also mappings
> > has to be 1-1. All requests to create a map where either the unmapped
> > or the mapped label already exists in the map will be denied.
> >
> > setns() with Smack namespace active has an additional check that the
> > label of a process that is calling setns() has to be already mapped in
> > the target Smack namespace for the call to succeed.
> >
> > --- Special labels ---
> >
> > Smack is using some special labels that have built-in rules. Things
> > like floor '_', dash '^', star '*', etc. Those labels are not
> > automatically mapped to the namespace. Moreover, you can choose to map
> > a different label from the init namespace to behave e.g. like floor
> > inside the namespace.
> >
> > Let's say we have no rules and those labels in the init namespace:
> > _
> > floor_to_be
> > label
> >
> > Both 'label' and 'floor_to_be' can read objects with '_'. But they
> > have no access rights to each other.
> >
> > Now let's create a map like this:
> > _ ordinary_label
> > floor_to_be _
> > label mapped
> >
> > Right now label 'mapped' can read label '_' which means that
> > effectively inside this namespace label 'label' has gained read access
> > to the 'floor_to_be'. The label 'ordinary_label' is exactly it, an
> > ordinary label that the built-in rules no longer apply to inside the
> > namespace.
> >
> > To sum up, special labels in the namespace behave the same as in the
> > init namespace. Not the original special labels though, but the ones
> > we map to specials. This is the only case where a namespace can have
> > access rights the init namespace does not have (like the 'label' to
> > 'floor_to_be' in the example above).
> >
> > Of course mappings like these are perfectly legal:
> > _ _
> > * *
> > ^ ^
> >
> >
> > --- Current limitations ---
> >
> > The Smack namespace is not hierarchical yet. It is currently not
> > possible to fill a smack_map of a nested user namespace (you can still
> > create nested user namespace, it will just inherit its parent's map
> > and won't have active Smack capabilities). When hierarchy will be
> > implemented the process creating another namespace will be allowed to
> > map only labels that it has permission to itself (those that it has in
> > its own map).
>
> > Special files inside the virtual smackfs needs to be reviewed whether
> > it's beneficial to have some of their functionality namespaced as well
> > (e.g. onlycap, syslog. ambient, etc). This would increase
> > CAP_MAC_ADMIN privileges inside the namespace.
> >
> >
> > Lukasz Pawelczyk (7):
> > user_ns: 3 new hooks for user namespace operations
> > smack: extend capability functions and fix 2 checks
> > smack: abstraction layer for 2 common Smack operations
> > smack: misc cleanups in preparation for a namespace patch
> > smack: namespace groundwork
> > smack: namespace implementation
> > smack: documentation for the Smack namespace
> >
> > Documentation/security/00-INDEX | 2 +
> > Documentation/security/Smack-namespace.txt | 231 +++++++++++++
> > MAINTAINERS | 1 +
> > fs/proc/base.c | 57 +++
> > include/linux/lsm_hooks.h | 28 ++
> > include/linux/security.h | 23 ++
> > include/linux/user_namespace.h | 9 +
> > kernel/user.c | 3 +
> > kernel/user_namespace.c | 18 +
> > security/security.c | 28 ++
> > security/smack/Kconfig | 12 +
> > security/smack/Makefile | 1 +
> > security/smack/smack.h | 187 +++++++++-
> > security/smack/smack_access.c | 191 ++++++++--
> > security/smack/smack_lsm.c | 536 ++++++++++++++++++++---------
> > security/smack/smack_ns.c | 471 +++++++++++++++++++++++++
> > security/smack/smackfs.c | 154 +++++----
> > 17 files changed, 1702 insertions(+), 250 deletions(-)
> > create mode 100644 Documentation/security/Smack-namespace.txt
> > create mode 100644 security/smack/smack_ns.c
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
^ 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