* Re: [PATCH 09/19] fs: Refuse uid/gid changes which don't map into s_user_ns
From: Seth Forshee @ 2015-12-04 17:46 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn,
Richard Weinberger, Austin S Hemmelgarn, Miklos Szeredi,
linux-bcache, dm-devel, linux-raid, linux-kernel, linux-mtd,
linux-fsdevel, fuse-devel, linux-security-module, selinux
In-Reply-To: <20151204172738.GA2280@mail.hallyn.com>
On Fri, Dec 04, 2015 at 11:27:38AM -0600, Serge E. Hallyn wrote:
> On Wed, Dec 02, 2015 at 09:40:09AM -0600, Seth Forshee wrote:
> > Add checks to inode_change_ok to verify that uid and gid changes
> > will map into the superblock's user namespace. If they do not
> > fail with -EOVERFLOW. This cannot be overriden with ATTR_FORCE.
> >
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>
> Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
>
> ... although i could see root on the host being upset that it can't
> assign a uid not valid in the mounter's ns. But it does seem safer.
That change wouldn't be representable in the backing store though, and
that could lead to unexpected behaviour. It's better to tell root that
we can't make the requested change, in my opinion.
^ permalink raw reply
* Re: RAID6 - "Invalid Argument" / "does not have a valid v1.2 superblock" ?!
From: Tobias Geiger @ 2015-12-04 17:32 UTC (permalink / raw)
To: linux-raid; +Cc: linux-raid-owner
In-Reply-To: <207014396.bPZ85pNCfm@t400>
Am 02.12.2015 18:04, schrieb Tobias Geiger:
> Hello,
>
> i need help assembling my raid6 again:
>
> root@pc:~# mdadm -A /dev/md0 -v --force /dev/sdd2 /dev/sde2 /dev/sdg2
> /dev/sdc2 /dev/sdb2 /dev/sdf2
> mdadm: looking for devices for /dev/md0
> mdadm: /dev/sdd2 is identified as a member of /dev/md0, slot 0.
> mdadm: /dev/sde2 is identified as a member of /dev/md0, slot 1.
> mdadm: /dev/sdg2 is identified as a member of /dev/md0, slot 2.
> mdadm: /dev/sdc2 is identified as a member of /dev/md0, slot 3.
> mdadm: /dev/sdb2 is identified as a member of /dev/md0, slot 4.
> mdadm: /dev/sdf2 is identified as a member of /dev/md0, slot 5.
> mdadm: failed to add /dev/sde2 to /dev/md0: Invalid argument
> mdadm: failed to add /dev/sdg2 to /dev/md0: Invalid argument
> mdadm: failed to add /dev/sdc2 to /dev/md0: Invalid argument
> mdadm: failed to add /dev/sdb2 to /dev/md0: Invalid argument
> mdadm: failed to add /dev/sdf2 to /dev/md0: Invalid argument
> mdadm: failed to add /dev/sdd2 to /dev/md0: Invalid argument
> mdadm: failed to RUN_ARRAY /dev/md0: Invalid argument
>
>
> dmesg says to this:
>
> [25717.423008] md: md0 stopped.
> [25717.424958] md: sde2 does not have a valid v1.2 superblock, not
> importing!
> [25717.424989] md: md_import_device returned -22
> [25717.425266] md: sdg2 does not have a valid v1.2 superblock, not
> importing!
> [25717.425963] md: md_import_device returned -22
> [25717.426847] md: sdc2 does not have a valid v1.2 superblock, not
> importing!
> [25717.426880] md: md_import_device returned -22
> [25717.427908] md: sdb2 does not have a valid v1.2 superblock, not
> importing!
> [25717.427952] md: md_import_device returned -22
> [25717.428890] md: sdf2 does not have a valid v1.2 superblock, not
> importing!
> [25717.428938] md: md_import_device returned -22
> [25717.429668] md: sdd2 does not have a valid v1.2 superblock, not
> importing!
> [25717.429716] md: md_import_device returned -22
> [25717.430104] md: md0 stopped.
>
>
> but i cant find any problems examining the devices:
>
>
>
> /dev/sdb2:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : 078883e8:30c2ebe6:87576034:e85737b8
> Name : pc:0 (local to host pc)
> Creation Time : Fri Sep 5 01:06:21 2014
> Raid Level : raid6
> Raid Devices : 6
>
> Avail Dev Size : 7813699215 (3725.86 GiB 4000.61 GB)
> Array Size : 15627124736 (14903.19 GiB 16002.18 GB)
> Used Dev Size : 7813562368 (3725.80 GiB 4000.54 GB)
> Data Offset : 198656 sectors
> Super Offset : 8 sectors
> Unused Space : before=198568 sectors, after=69263 sectors
> State : clean
> Device UUID : 02dab3ff:6e36ebf3:2012e2a0:e8b29f3f
>
> Update Time : Wed Dec 2 10:13:53 2015
> Bad Block Log : 512 entries available at offset 72 sectors
> Checksum : b75c2285 - correct
> Events : 1903827
>
> Layout : left-symmetric
> Chunk Size : 4096K
>
> Device Role : Active device 4
> Array State : AAAAAA ('A' == active, '.' == missing, 'R' ==
> replacing)
> /dev/sdc2:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : 078883e8:30c2ebe6:87576034:e85737b8
> Name : pc:0 (local to host pc)
> Creation Time : Fri Sep 5 01:06:21 2014
> Raid Level : raid6
> Raid Devices : 6
>
> Avail Dev Size : 7813699215 (3725.86 GiB 4000.61 GB)
> Array Size : 15627124736 (14903.19 GiB 16002.18 GB)
> Used Dev Size : 7813562368 (3725.80 GiB 4000.54 GB)
> Data Offset : 198656 sectors
> Super Offset : 8 sectors
> Unused Space : before=198568 sectors, after=69263 sectors
> State : clean
> Device UUID : c9aaa3e3:00589ba5:be8fe774:362d477a
>
> Update Time : Wed Dec 2 10:13:53 2015
> Bad Block Log : 512 entries available at offset 72 sectors
> Checksum : 5ba906b9 - correct
> Events : 1903827
>
> Layout : left-symmetric
> Chunk Size : 4096K
>
> Device Role : Active device 3
> Array State : AAAAAA ('A' == active, '.' == missing, 'R' ==
> replacing)
> /dev/sdd2:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : 078883e8:30c2ebe6:87576034:e85737b8
> Name : pc:0 (local to host pc)
> Creation Time : Fri Sep 5 01:06:21 2014
> Raid Level : raid6
> Raid Devices : 6
>
> Avail Dev Size : 7813699215 (3725.86 GiB 4000.61 GB)
> Array Size : 15627124736 (14903.19 GiB 16002.18 GB)
> Used Dev Size : 7813562368 (3725.80 GiB 4000.54 GB)
> Data Offset : 198656 sectors
> Super Offset : 8 sectors
> Unused Space : before=198568 sectors, after=69263 sectors
> State : clean
> Device UUID : 2b4e613e:1a7f062c:f8ba29a9:e5dcfaf8
>
> Update Time : Wed Dec 2 10:13:53 2015
> Bad Block Log : 512 entries available at offset 72 sectors
> Checksum : efc7b1df - correct
> Events : 1903827
>
> Layout : left-symmetric
> Chunk Size : 4096K
>
> Device Role : Active device 0
> Array State : AAAAAA ('A' == active, '.' == missing, 'R' ==
> replacing)
> /dev/sde2:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : 078883e8:30c2ebe6:87576034:e85737b8
> Name : pc:0 (local to host pc)
> Creation Time : Fri Sep 5 01:06:21 2014
> Raid Level : raid6
> Raid Devices : 6
>
> Avail Dev Size : 7813699215 (3725.86 GiB 4000.61 GB)
> Array Size : 15627124736 (14903.19 GiB 16002.18 GB)
> Used Dev Size : 7813562368 (3725.80 GiB 4000.54 GB)
> Data Offset : 198656 sectors
> Super Offset : 8 sectors
> Unused Space : before=198568 sectors, after=69263 sectors
> State : clean
> Device UUID : e80ba505:cc0b9904:9288c64a:5c289e62
>
> Update Time : Wed Dec 2 10:13:53 2015
> Bad Block Log : 512 entries available at offset 72 sectors
> Checksum : 9ade0f9a - correct
> Events : 1903827
>
> Layout : left-symmetric
> Chunk Size : 4096K
>
> Device Role : Active device 1
> Array State : AAAAAA ('A' == active, '.' == missing, 'R' ==
> replacing)
> /dev/sdf2:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : 078883e8:30c2ebe6:87576034:e85737b8
> Name : pc:0 (local to host pc)
> Creation Time : Fri Sep 5 01:06:21 2014
> Raid Level : raid6
> Raid Devices : 6
>
> Avail Dev Size : 7813699215 (3725.86 GiB 4000.61 GB)
> Array Size : 15627124736 (14903.19 GiB 16002.18 GB)
> Used Dev Size : 7813562368 (3725.80 GiB 4000.54 GB)
> Data Offset : 198656 sectors
> Super Offset : 8 sectors
> Unused Space : before=198568 sectors, after=69263 sectors
> State : clean
> Device UUID : f769eefd:bd202fca:50966083:39a25647
>
> Update Time : Wed Dec 2 10:13:53 2015
> Bad Block Log : 512 entries available at offset 72 sectors
> Checksum : 76100a3c - correct
> Events : 1903827
>
> Layout : left-symmetric
> Chunk Size : 4096K
>
> Device Role : Active device 5
> Array State : AAAAAA ('A' == active, '.' == missing, 'R' ==
> replacing)
> /dev/sdg2:
> Magic : a92b4efc
> Version : 1.2
> Feature Map : 0x0
> Array UUID : 078883e8:30c2ebe6:87576034:e85737b8
> Name : pc:0 (local to host pc)
> Creation Time : Fri Sep 5 01:06:21 2014
> Raid Level : raid6
> Raid Devices : 6
>
> Avail Dev Size : 7813699215 (3725.86 GiB 4000.61 GB)
> Array Size : 15627124736 (14903.19 GiB 16002.18 GB)
> Used Dev Size : 7813562368 (3725.80 GiB 4000.54 GB)
> Data Offset : 198656 sectors
> Super Offset : 8 sectors
> Unused Space : before=198568 sectors, after=69263 sectors
> State : clean
> Device UUID : 7fad3e2f:85dd6849:127d7d47:58c166dd
>
> Update Time : Wed Dec 2 10:13:53 2015
> Bad Block Log : 512 entries available at offset 72 sectors
> Checksum : 80c71068 - correct
> Events : 1903827
>
> Layout : left-symmetric
> Chunk Size : 4096K
>
> Device Role : Active device 2
> Array State : AAAAAA ('A' == active, '.' == missing, 'R' ==
> replacing)
>
>
>
> except for the Checksum and the Device UUID the output is exactly the
> same for
> each device...
>
>
> i really hesitate to "recreate" the array with "--assume-clean",
> because of
> the Unused-Space/ArraySize, which i wouldnt know how to reassemble in a
> "mdadm --create ..." line ...
>
>
> do you have any thoughts on that?
> Thanks very much for your help!
>
> Greetings
> Tobias
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
I convinced myself that there is no other chance, and did:
mdadm -v --create /dev/md0 --readonly --bitmap=none --chunk=4M --level=6
--raid-device=6 --assume-clean --data-offset 99328 /dev/sdd2 /dev/sde2
/dev/sdg2 /dev/sdc2 /dev/sdb2 /dev/sdf2
that worked out (puh!), so it seems that "only" the data-offset (and of
course chunk/level...) needed to match.
That is btw in "sectors" in the --examine output, so one has to convert
it to kbytes for the --create line!
Greetings!
Tobias
^ permalink raw reply
* Re: [PATCH 09/19] fs: Refuse uid/gid changes which don't map into s_user_ns
From: Serge E. Hallyn @ 2015-12-04 17:27 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn,
Richard Weinberger, Austin S Hemmelgarn, Miklos Szeredi,
linux-bcache, dm-devel, linux-raid, linux-kernel, linux-mtd,
linux-fsdevel, fuse-devel, linux-security-module, selinux
In-Reply-To: <1449070821-73820-10-git-send-email-seth.forshee@canonical.com>
On Wed, Dec 02, 2015 at 09:40:09AM -0600, Seth Forshee wrote:
> Add checks to inode_change_ok to verify that uid and gid changes
> will map into the superblock's user namespace. If they do not
> fail with -EOVERFLOW. This cannot be overriden with ATTR_FORCE.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
... although i could see root on the host being upset that it can't
assign a uid not valid in the mounter's ns. But it does seem safer.
> ---
> fs/attr.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/fs/attr.c b/fs/attr.c
> index 6530ced19697..55b46e3aa888 100644
> --- a/fs/attr.c
> +++ b/fs/attr.c
> @@ -42,6 +42,17 @@ int inode_change_ok(const struct inode *inode, struct iattr *attr)
> return error;
> }
>
> + /*
> + * Verify that uid/gid changes are valid in the target namespace
> + * of the superblock. This cannot be overriden using ATTR_FORCE.
> + */
> + if (ia_valid & ATTR_UID &&
> + from_kuid(inode->i_sb->s_user_ns, attr->ia_uid) == (uid_t)-1)
> + return -EOVERFLOW;
> + if (ia_valid & ATTR_GID &&
> + from_kgid(inode->i_sb->s_user_ns, attr->ia_gid) == (gid_t)-1)
> + return -EOVERFLOW;
> +
> /* If force is set do it anyway. */
> if (ia_valid & ATTR_FORCE)
> return 0;
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 05/19] userns: Replace in_userns with current_in_userns
From: Serge E. Hallyn @ 2015-12-04 17:01 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn, James Morris,
Serge E. Hallyn, Richard Weinberger, Austin S Hemmelgarn,
Miklos Szeredi, linux-bcache, dm-devel, linux-raid, linux-kernel,
linux-mtd, linux-fsdevel, fuse-devel, linux-security-module,
selinux
In-Reply-To: <1449070821-73820-6-git-send-email-seth.forshee@canonical.com>
On Wed, Dec 02, 2015 at 09:40:05AM -0600, Seth Forshee wrote:
> All current callers of in_userns pass current_user_ns as the
> first argument. Simplify by replacing in_userns with
> current_in_userns which checks whether current_user_ns is in the
> namespace supplied as an argument.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> Acked-by: James Morris <james.l.morris@oracle.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> fs/namespace.c | 2 +-
> include/linux/user_namespace.h | 6 ++----
> kernel/user_namespace.c | 6 +++---
> security/commoncap.c | 2 +-
> 4 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 2101ce7b96ab..18fc58760aec 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3286,7 +3286,7 @@ bool mnt_may_suid(struct vfsmount *mnt)
> * in other namespaces.
> */
> return !(mnt->mnt_flags & MNT_NOSUID) && check_mnt(real_mount(mnt)) &&
> - in_userns(current_user_ns(), mnt->mnt_sb->s_user_ns);
> + current_in_userns(mnt->mnt_sb->s_user_ns);
> }
>
> static struct ns_common *mntns_get(struct task_struct *task)
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index a43faa727124..9217169c64cb 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -72,8 +72,7 @@ extern ssize_t proc_projid_map_write(struct file *, const char __user *, size_t,
> extern ssize_t proc_setgroups_write(struct file *, const char __user *, size_t, loff_t *);
> extern int proc_setgroups_show(struct seq_file *m, void *v);
> extern bool userns_may_setgroups(const struct user_namespace *ns);
> -extern bool in_userns(const struct user_namespace *ns,
> - const struct user_namespace *target_ns);
> +extern bool current_in_userns(const struct user_namespace *target_ns);
> #else
>
> static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
> @@ -103,8 +102,7 @@ static inline bool userns_may_setgroups(const struct user_namespace *ns)
> return true;
> }
>
> -static inline bool in_userns(const struct user_namespace *ns,
> - const struct user_namespace *target_ns)
> +static inline bool current_in_userns(const struct user_namespace *target_ns)
> {
> return true;
> }
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 69fbc377357b..5960edc7e644 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -949,10 +949,10 @@ bool userns_may_setgroups(const struct user_namespace *ns)
> * Returns true if @ns is the same namespace as or a descendant of
> * @target_ns.
> */
> -bool in_userns(const struct user_namespace *ns,
> - const struct user_namespace *target_ns)
> +bool current_in_userns(const struct user_namespace *target_ns)
> {
> - for (; ns; ns = ns->parent) {
> + struct user_namespace *ns;
> + for (ns = current_user_ns(); ns; ns = ns->parent) {
> if (ns == target_ns)
> return true;
> }
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6243aef5860e..2119421613f6 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -450,7 +450,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_c
>
> if (!mnt_may_suid(bprm->file->f_path.mnt))
> return 0;
> - if (!in_userns(current_user_ns(), bprm->file->f_path.mnt->mnt_sb->s_user_ns))
> + if (!current_in_userns(bprm->file->f_path.mnt->mnt_sb->s_user_ns))
> return 0;
>
> rc = get_vfs_caps_from_disk(bprm->file->f_path.dentry, &vcaps);
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH 03/19] fs: Treat foreign mounts as nosuid
From: Serge E. Hallyn @ 2015-12-04 16:55 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn, James Morris,
Serge E. Hallyn, Paul Moore, Stephen Smalley, Eric Paris,
Richard Weinberger, Austin S Hemmelgarn, Miklos Szeredi,
linux-bcache, dm-devel, linux-raid, linux-kernel, linux-mtd,
linux-fsdevel, fuse-devel, linux-security-module, selinux,
Andy Lutomirski
In-Reply-To: <1449070821-73820-4-git-send-email-seth.forshee@canonical.com>
On Wed, Dec 02, 2015 at 09:40:03AM -0600, Seth Forshee wrote:
> From: Andy Lutomirski <luto@amacapital.net>
>
> If a process gets access to a mount from a different user
> namespace, that process should not be able to take advantage of
> setuid files or selinux entrypoints from that filesystem. Prevent
> this by treating mounts from other mount namespaces and those not
> owned by current_user_ns() or an ancestor as nosuid.
>
> This will make it safer to allow more complex filesystems to be
> mounted in non-root user namespaces.
>
> This does not remove the need for MNT_LOCK_NOSUID. The setuid,
> setgid, and file capability bits can no longer be abused if code in
> a user namespace were to clear nosuid on an untrusted filesystem,
> but this patch, by itself, is insufficient to protect the system
> from abuse of files that, when execed, would increase MAC privilege.
>
> As a more concrete explanation, any task that can manipulate a
> vfsmount associated with a given user namespace already has
> capabilities in that namespace and all of its descendents. If they
> can cause a malicious setuid, setgid, or file-caps executable to
> appear in that mount, then that executable will only allow them to
> elevate privileges in exactly the set of namespaces in which they
> are already privileges.
>
> On the other hand, if they can cause a malicious executable to
> appear with a dangerous MAC label, running it could change the
> caller's security context in a way that should not have been
> possible, even inside the namespace in which the task is confined.
>
> As a hardening measure, this would have made CVE-2014-5207 much
> more difficult to exploit.
>
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> Acked-by: James Morris <james.l.morris@oracle.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> fs/exec.c | 2 +-
> fs/namespace.c | 13 +++++++++++++
> include/linux/mount.h | 1 +
> security/commoncap.c | 2 +-
> security/selinux/hooks.c | 2 +-
> 5 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index b06623a9347f..ea7311d72cc3 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1295,7 +1295,7 @@ static void bprm_fill_uid(struct linux_binprm *bprm)
> bprm->cred->euid = current_euid();
> bprm->cred->egid = current_egid();
>
> - if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
> + if (!mnt_may_suid(bprm->file->f_path.mnt))
> return;
>
> if (task_no_new_privs(current))
> diff --git a/fs/namespace.c b/fs/namespace.c
> index da70f7c4ece1..2101ce7b96ab 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3276,6 +3276,19 @@ found:
> return visible;
> }
>
> +bool mnt_may_suid(struct vfsmount *mnt)
> +{
> + /*
> + * Foreign mounts (accessed via fchdir or through /proc
> + * symlinks) are always treated as if they are nosuid. This
> + * prevents namespaces from trusting potentially unsafe
> + * suid/sgid bits, file caps, or security labels that originate
> + * in other namespaces.
> + */
> + return !(mnt->mnt_flags & MNT_NOSUID) && check_mnt(real_mount(mnt)) &&
> + in_userns(current_user_ns(), mnt->mnt_sb->s_user_ns);
> +}
> +
> static struct ns_common *mntns_get(struct task_struct *task)
> {
> struct ns_common *ns = NULL;
> diff --git a/include/linux/mount.h b/include/linux/mount.h
> index f822c3c11377..54a594d49733 100644
> --- a/include/linux/mount.h
> +++ b/include/linux/mount.h
> @@ -81,6 +81,7 @@ extern void mntput(struct vfsmount *mnt);
> extern struct vfsmount *mntget(struct vfsmount *mnt);
> extern struct vfsmount *mnt_clone_internal(struct path *path);
> extern int __mnt_is_readonly(struct vfsmount *mnt);
> +extern bool mnt_may_suid(struct vfsmount *mnt);
>
> struct path;
> extern struct vfsmount *clone_private_mount(struct path *path);
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 400aa224b491..6243aef5860e 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -448,7 +448,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_c
> if (!file_caps_enabled)
> return 0;
>
> - if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
> + if (!mnt_may_suid(bprm->file->f_path.mnt))
> return 0;
> if (!in_userns(current_user_ns(), bprm->file->f_path.mnt->mnt_sb->s_user_ns))
> return 0;
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d0cfaa9f19d0..a5b93df6553f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2171,7 +2171,7 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm,
> const struct task_security_struct *new_tsec)
> {
> int nnp = (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS);
> - int nosuid = (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID);
> + int nosuid = !mnt_may_suid(bprm->file->f_path.mnt);
> int rc;
>
> if (!nnp && !nosuid)
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH 08/19] cred: Reject inodes with invalid ids in set_create_file_as()
From: Serge E. Hallyn @ 2015-12-04 16:49 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn,
Richard Weinberger, Austin S Hemmelgarn, Miklos Szeredi,
linux-bcache, dm-devel, linux-raid, linux-kernel, linux-mtd,
linux-fsdevel, fuse-devel, linux-security-module, selinux
In-Reply-To: <1449070821-73820-9-git-send-email-seth.forshee@canonical.com>
On Wed, Dec 02, 2015 at 09:40:08AM -0600, Seth Forshee wrote:
> Using INVALID_[UG]ID for the LSM file creation context doesn't
> make sense, so return an error if the inode passed to
> set_create_file_as() has an invalid id.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> kernel/cred.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 71179a09c1d6..ff8606f77d90 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -689,6 +689,8 @@ EXPORT_SYMBOL(set_security_override_from_ctx);
> */
> int set_create_files_as(struct cred *new, struct inode *inode)
> {
> + if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
> + return -EINVAL;
> new->fsuid = inode->i_uid;
> new->fsgid = inode->i_gid;
> return security_kernel_create_files_as(new, inode);
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 07/19] fs: Check for invalid i_uid in may_follow_link()
From: Serge E. Hallyn @ 2015-12-04 16:42 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn,
Richard Weinberger, Austin S Hemmelgarn, Miklos Szeredi,
linux-bcache, dm-devel, linux-raid, linux-kernel, linux-mtd,
linux-fsdevel, fuse-devel, linux-security-module, selinux
In-Reply-To: <1449070821-73820-8-git-send-email-seth.forshee@canonical.com>
On Wed, Dec 02, 2015 at 09:40:07AM -0600, Seth Forshee wrote:
> Filesystem uids which don't map into a user namespace may result
> in inode->i_uid being INVALID_UID. A symlink and its parent
> could have different owners in the filesystem can both get
> mapped to INVALID_UID, which may result in following a symlink
> when this would not have otherwise been permitted when protected
> symlinks are enabled.
>
> Add a new helper function, uid_valid_eq(), and use this to
> validate that the ids in may_follow_link() are both equal and
> valid. Also add an equivalent helper for gids, which is
> currently unused.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> fs/namei.c | 2 +-
> include/linux/uidgid.h | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 288e8a74bf88..4ccafd391697 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -902,7 +902,7 @@ static inline int may_follow_link(struct nameidata *nd)
> return 0;
>
> /* Allowed if parent directory and link owner match. */
> - if (uid_eq(parent->i_uid, inode->i_uid))
> + if (uid_valid_eq(parent->i_uid, inode->i_uid))
> return 0;
>
> if (nd->flags & LOOKUP_RCU)
> diff --git a/include/linux/uidgid.h b/include/linux/uidgid.h
> index 03835522dfcb..e09529fe2668 100644
> --- a/include/linux/uidgid.h
> +++ b/include/linux/uidgid.h
> @@ -117,6 +117,16 @@ static inline bool gid_valid(kgid_t gid)
> return __kgid_val(gid) != (gid_t) -1;
> }
>
> +static inline bool uid_valid_eq(kuid_t left, kuid_t right)
> +{
> + return uid_eq(left, right) && uid_valid(left);
> +}
> +
> +static inline bool gid_valid_eq(kgid_t left, kgid_t right)
> +{
> + return gid_eq(left, right) && gid_valid(left);
> +}
> +
> #ifdef CONFIG_USER_NS
>
> extern kuid_t make_kuid(struct user_namespace *from, uid_t uid);
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 02/19] block_dev: Check permissions towards block device inode when mounting
From: Serge E. Hallyn @ 2015-12-04 16:28 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn,
Richard Weinberger, Austin S Hemmelgarn, Miklos Szeredi,
linux-bcache, dm-devel, linux-raid, linux-kernel, linux-mtd,
linux-fsdevel, fuse-devel, linux-security-module, selinux
In-Reply-To: <1449070821-73820-3-git-send-email-seth.forshee@canonical.com>
On Wed, Dec 02, 2015 at 09:40:02AM -0600, Seth Forshee wrote:
> Unprivileged users should not be able to mount block devices when
> they lack sufficient privileges towards the block device inode.
> Update blkdev_get_by_path() to validate that the user has the
> required access to the inode at the specified path. The check
> will be skipped for CAP_SYS_ADMIN, so privileged mounts will
> continue working as before.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> fs/block_dev.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 3ebbde85d898..4fdb6ab59816 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1424,9 +1424,14 @@ struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,
> void *holder)
> {
> struct block_device *bdev;
> + int perm = 0;
> int err;
>
> - bdev = lookup_bdev(path, 0);
> + if (mode & FMODE_READ)
> + perm |= MAY_READ;
> + if (mode & FMODE_WRITE)
> + perm |= MAY_WRITE;
> + bdev = lookup_bdev(path, perm);
> if (IS_ERR(bdev))
> return bdev;
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 01/19] block_dev: Support checking inode permissions in lookup_bdev()
From: Serge E. Hallyn @ 2015-12-04 16:26 UTC (permalink / raw)
To: Seth Forshee
Cc: Mike Snitzer, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
J. Bruce Fields, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
selinux-+05T5uksL2qpZYMLLGbcSA, Jeff Layton, Alasdair Kergon,
Miklos Szeredi, Austin S Hemmelgarn,
fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Kent Overstreet,
Serge Hallyn, linux-raid-u79uwXL29TY76Z2rM5mHXA,
linux-bcache-u79uwXL29TY76Z2rM5mHXA, Alexander Viro, Neil Brown,
linux-security-module-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
Jan Kara, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Brian Norris,
David Woodhouse
In-Reply-To: <1449070821-73820-2-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
On Wed, Dec 02, 2015 at 09:40:01AM -0600, Seth Forshee wrote:
> When looking up a block device by path no permission check is
> done to verify that the user has access to the block device inode
> at the specified path. In some cases it may be necessary to
> check permissions towards the inode, such as allowing
> unprivileged users to mount block devices in user namespaces.
>
> Add an argument to lookup_bdev() to optionally perform this
> permission check. A value of 0 skips the permission check and
> behaves the same as before. A non-zero value specifies the mask
> of access rights required towards the inode at the specified
> path. The check is always skipped if the user has CAP_SYS_ADMIN.
>
> All callers of lookup_bdev() currently pass a mask of 0, so this
> patch results in no functional change. Subsequent patches will
> add permission checks where appropriate.
>
> Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
> drivers/md/bcache/super.c | 2 +-
> drivers/md/dm-table.c | 2 +-
> drivers/mtd/mtdsuper.c | 2 +-
> fs/block_dev.c | 13 ++++++++++---
> fs/quota/quota.c | 2 +-
> include/linux/fs.h | 2 +-
> 6 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 679a093a3bf6..e8287b0d1dac 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -1926,7 +1926,7 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
> sb);
> if (IS_ERR(bdev)) {
> if (bdev == ERR_PTR(-EBUSY)) {
> - bdev = lookup_bdev(strim(path));
> + bdev = lookup_bdev(strim(path), 0);
> mutex_lock(&bch_register_lock);
> if (!IS_ERR(bdev) && bch_is_open(bdev))
> err = "device already registered";
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 061152a43730..81c60b2495ed 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -380,7 +380,7 @@ int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode,
> BUG_ON(!t);
>
> /* convert the path to a device */
> - bdev = lookup_bdev(path);
> + bdev = lookup_bdev(path, 0);
> if (IS_ERR(bdev)) {
> dev = name_to_dev_t(path);
> if (!dev)
> diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
> index 20c02a3b7417..b5b60e1af31c 100644
> --- a/drivers/mtd/mtdsuper.c
> +++ b/drivers/mtd/mtdsuper.c
> @@ -176,7 +176,7 @@ struct dentry *mount_mtd(struct file_system_type *fs_type, int flags,
> /* try the old way - the hack where we allowed users to mount
> * /dev/mtdblock$(n) but didn't actually _use_ the blockdev
> */
> - bdev = lookup_bdev(dev_name);
> + bdev = lookup_bdev(dev_name, 0);
> if (IS_ERR(bdev)) {
> ret = PTR_ERR(bdev);
> pr_debug("MTDSB: lookup_bdev() returned %d\n", ret);
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index f90d91efa1b4..3ebbde85d898 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1426,7 +1426,7 @@ struct block_device *blkdev_get_by_path(const char *path, fmode_t mode,
> struct block_device *bdev;
> int err;
>
> - bdev = lookup_bdev(path);
> + bdev = lookup_bdev(path, 0);
> if (IS_ERR(bdev))
> return bdev;
>
> @@ -1736,12 +1736,14 @@ EXPORT_SYMBOL(ioctl_by_bdev);
> /**
> * lookup_bdev - lookup a struct block_device by name
> * @pathname: special file representing the block device
> + * @mask: rights to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
> *
> * Get a reference to the blockdevice at @pathname in the current
> * namespace if possible and return it. Return ERR_PTR(error)
> - * otherwise.
> + * otherwise. If @mask is non-zero, check for access rights to the
> + * inode at @pathname.
> */
> -struct block_device *lookup_bdev(const char *pathname)
> +struct block_device *lookup_bdev(const char *pathname, int mask)
> {
> struct block_device *bdev;
> struct inode *inode;
> @@ -1756,6 +1758,11 @@ struct block_device *lookup_bdev(const char *pathname)
> return ERR_PTR(error);
>
> inode = d_backing_inode(path.dentry);
> + if (mask != 0 && !capable(CAP_SYS_ADMIN)) {
> + error = __inode_permission(inode, mask);
> + if (error)
> + goto fail;
> + }
> error = -ENOTBLK;
> if (!S_ISBLK(inode->i_mode))
> goto fail;
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 3746367098fd..a40eaecbd5cc 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -733,7 +733,7 @@ static struct super_block *quotactl_block(const char __user *special, int cmd)
>
> if (IS_ERR(tmp))
> return ERR_CAST(tmp);
> - bdev = lookup_bdev(tmp->name);
> + bdev = lookup_bdev(tmp->name, 0);
> putname(tmp);
> if (IS_ERR(bdev))
> return ERR_CAST(bdev);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8a17c5649ef2..879ec382fd88 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2373,7 +2373,7 @@ static inline void unregister_chrdev(unsigned int major, const char *name)
> #define BLKDEV_MAJOR_HASH_SIZE 255
> extern const char *__bdevname(dev_t, char *buffer);
> extern const char *bdevname(struct block_device *bdev, char *buffer);
> -extern struct block_device *lookup_bdev(const char *);
> +extern struct block_device *lookup_bdev(const char *, int mask);
> extern void blkdev_show(struct seq_file *,off_t);
>
> #else
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
------------------------------------------------------------------------------
Go from Idea to Many App Stores Faster with Intel(R) XDK
Give your users amazing mobile app experiences with Intel(R) XDK.
Use one codebase in this all-in-one HTML5 development environment.
Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs.
http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
^ permalink raw reply
* Re: RAID 5,6 sequential writing seems slower in newer kernels
From: Dallas Clement @ 2015-12-04 16:08 UTC (permalink / raw)
To: Robert Kierski; +Cc: Phil Turmel, linux-raid@vger.kernel.org
In-Reply-To: <F7761B9B1D11B64BBB666019E9378117FDE157@CFWEX01.americas.cray.com>
On Fri, Dec 4, 2015 at 7:40 AM, Robert Kierski <rkierski@cray.com> wrote:
> It turns out the problem I'm experiencing is related to thread count. When I run XDD with a reasonable queuedepth parameter (32), I get horrible performance. When I run it with a small queuedepth (1-4), I get expected performance.
>
> Here are the command lines:
>
> Horrible Performance:
> xdd -id commandline -dio -maxall -targets 1 /dev/md0 -queuedepth 32 -blocksize 1048576 -timelimit 10 -reqsize 1 -mbytes 5000 -passes 20 -verbose -op write -seek sequential
>
> GOOD Performance:
> xdd -id commandline -dio -maxall -targets 1 /dev/md0 -queuedepth 1 -blocksize 1048576 -timelimit 10 -reqsize 1 -mbytes 5000 -passes 20 -verbose -op write -seek sequential
>
> BEST Performance:
> xdd -id commandline -dio -maxall -targets 1 /dev/md0 -queuedepth 3 -blocksize 1048576 -timelimit 10 -reqsize 1 -mbytes 5000 -passes 20 -verbose -op write -seek sequential
>
> BAD Performance
> xdd -id commandline -dio -maxall -targets 1 /dev/md1 -queuedepth 5 -blocksize 1048576 -timelimit 10 -reqsize 1 -mbytes 5000 -passes 20 -verbose -op write -seek sequential
>
> Bob Kierski
> Senior Storage Performance Engineer
> Cray Inc.
> 380 Jackson Street
> Suite 210
> St. Paul, MN 55101
> Tele: 651-967-9590
> Fax: 651-605-9001
> Cell: 651-890-7461
>
Robert, is this with a RAID 6 array? Also, what are the actual
throughput numbers you are getting? Would it be possible for you to
capture the iostat -xm output and report the individual disk wMB/s and
also the disk utilization for RAID 6?
^ permalink raw reply
* Re: [PATCH 17/19] fuse: Support fuse filesystems outside of init_user_ns
From: Seth Forshee @ 2015-12-04 15:38 UTC (permalink / raw)
To: Eric W. Biederman, Miklos Szeredi
Cc: Alexander Viro, Serge Hallyn, Richard Weinberger,
Austin S Hemmelgarn, linux-bcache, dm-devel, linux-raid,
linux-kernel, linux-mtd, linux-fsdevel, fuse-devel,
linux-security-module, selinux
In-Reply-To: <1449070821-73820-18-git-send-email-seth.forshee@canonical.com>
On Wed, Dec 02, 2015 at 09:40:17AM -0600, Seth Forshee wrote:
> @@ -155,11 +155,22 @@ static ino_t fuse_squash_ino(u64 ino64)
> return ino;
> }
>
> -void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
> - u64 attr_valid)
> +int fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
> + u64 attr_valid)
> {
> struct fuse_conn *fc = get_fuse_conn(inode);
> struct fuse_inode *fi = get_fuse_inode(inode);
> + kuid_t uid;
> + kgid_t gid;
> +
> + uid = make_kuid(fc->user_ns, attr->uid);
> + gid = make_kgid(fc->user_ns, attr->gid);
> + if (!uid_valid(uid) || !gid_valid(gid)) {
> + make_bad_inode(inode);
> + return -EIO;
> + }
Eric - I had kind of forgotten about this part until just now, but
previously with these patches we had discussed how to handle ids from
the filesystem that aren't valid in s_user_ns. My intention is to set
the kuids in the inode to invalid, and in these patches I've updated the
vfs so that it should be safe to do that. But at some point I think you
had suggested marking the inodes bad, and I must have added this as a
result. I guess we need to decide which way to go. I favor using invalid
ids so that a user privileged in s_user_ns can still access the inode,
change ownership, etc., but I'm interested to hear your opinion.
Thanks,
Seth
^ permalink raw reply
* RE: RAID 5,6 sequential writing seems slower in newer kernels
From: Robert Kierski @ 2015-12-04 13:40 UTC (permalink / raw)
To: Phil Turmel, Dallas Clement, linux-raid@vger.kernel.org
In-Reply-To: <566059E8.60804@turmel.org>
It turns out the problem I'm experiencing is related to thread count. When I run XDD with a reasonable queuedepth parameter (32), I get horrible performance. When I run it with a small queuedepth (1-4), I get expected performance.
Here are the command lines:
Horrible Performance:
xdd -id commandline -dio -maxall -targets 1 /dev/md0 -queuedepth 32 -blocksize 1048576 -timelimit 10 -reqsize 1 -mbytes 5000 -passes 20 -verbose -op write -seek sequential
GOOD Performance:
xdd -id commandline -dio -maxall -targets 1 /dev/md0 -queuedepth 1 -blocksize 1048576 -timelimit 10 -reqsize 1 -mbytes 5000 -passes 20 -verbose -op write -seek sequential
BEST Performance:
xdd -id commandline -dio -maxall -targets 1 /dev/md0 -queuedepth 3 -blocksize 1048576 -timelimit 10 -reqsize 1 -mbytes 5000 -passes 20 -verbose -op write -seek sequential
BAD Performance
xdd -id commandline -dio -maxall -targets 1 /dev/md1 -queuedepth 5 -blocksize 1048576 -timelimit 10 -reqsize 1 -mbytes 5000 -passes 20 -verbose -op write -seek sequential
Bob Kierski
Senior Storage Performance Engineer
Cray Inc.
380 Jackson Street
Suite 210
St. Paul, MN 55101
Tele: 651-967-9590
Fax: 651-605-9001
Cell: 651-890-7461
^ permalink raw reply
* Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-12-04 4:58 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Mark Brown, Jens Axboe, keith.busch, Jan Kara, Arnd Bergmann,
Mike Snitzer, neilb, LKML, linux-raid, dm-devel, Garg, Dinesh, tj,
bart.vanassche, jmoyer, Alasdair G Kergon
In-Reply-To: <alpine.LRH.2.02.1512031047340.15134@file01.intranet.prod.int.rdu2.redhat.com>
On 3 December 2015 at 23:49, Mikulas Patocka <mpatocka@redhat.com> wrote:
>
>
> On Thu, 3 Dec 2015, Baolin Wang wrote:
>
>> On 3 December 2015 at 10:56, Baolin Wang <baolin.wang@linaro.org> wrote:
>> > On 3 December 2015 at 03:56, Alasdair G Kergon <agk@redhat.com> wrote:
>> >> On Wed, Dec 02, 2015 at 08:46:54PM +0800, Baolin Wang wrote:
>> >>> These are the benchmarks for request based dm-crypt. Please check it.
>> >>
>> >> Now please put request-based dm-crypt completely to one side and focus
>> >> just on the existing bio-based code. Why is it slower and what can be
>> >> adjusted to improve this?
>> >>
>> >
>> > OK. I think I find something need to be point out.
>> > 1. From the IO block size test in the performance report, for the
>> > request based, we can find it can not get the corresponding
>> > performance if we just expand the IO size. Because In dm crypt, it
>> > will map the data buffer of one request with scatterlists, and send
>> > all scatterlists of one request to the encryption engine to encrypt or
>> > decrypt. I found if the scatterlist list number is small and each
>> > scatterlist length is bigger, it will improve the encryption speed,
>> > that helps the engine palys best performance. But a big IO size does
>> > not mean bigger scatterlists (maybe many scatterlists with small
>> > length), that's why we can not get the corresponding performance if we
>> > just expand the IO size I think.
>> >
>> > 2. Why bio based is slower?
>> > If you understand 1, you can obviously understand the crypto engine
>> > likes bigger scatterlists to improve the performance. But for bio
>> > based, it only send one scatterlist (the scatterlist's length is
>> > always '1 << SECTOR_SHIFT' = 512) to the crypto engine at one time. It
>> > means if the bio size is 1M, the bio based will send 2048 times (evey
>> > time the only one scatterlist length is 512 bytes) to crypto engine to
>> > handle, which is more time-consuming and ineffective for the crypto
>> > engine. But for request based, it can map the whole request with many
>> > scatterlists (not just one scatterlist), and send all the scatterlists
>> > to the crypto engine which can improve the performance, is it right?
>> >
>> > Another optimization solution I think is we can expand the scatterlist
>> > entry number for bio based.
>> >
>>
>> I did some testing about my assumption of expanding the scatterlist
>> entry number for bio based. I did some modification for the bio based
>> to support multiple scatterlists, then it will get the same
>> performance as the request based things.
>>
>> 1. bio based with expanding the scatterlist entry
>> time dd if=/dev/dm-0 of=/dev/null bs=64K count=16384 iflag=direct
>> 1073741824 bytes (1.1 GB) copied, 94.5458 s, 11.4 MB/s
>> real 1m34.562s
>> user 0m0.030s
>> sys 0m3.850s
>>
>> 2. Sequential read 1G with requset based:
>> time dd if=/dev/dm-0 of=/dev/null bs=64K count=16384 iflag=direct
>> 1073741824 bytes (1.1 GB) copied, 94.8922 s, 11.3 MB/s
>> real 1m34.908s
>> user 0m0.030s
>> sys 0m4.000s
>
> Measuring the system time this way is completely wrong because it doesn't
> account for the time spent in kernel threads.
>
OK. Thanks for your suggestions.
> Mikulas
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-12-04 4:57 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Mark Brown, Jens Axboe, keith.busch, Jan Kara, Arnd Bergmann,
Mike Snitzer, neilb, LKML, linux-raid, dm-devel, Garg, Dinesh, tj,
bart.vanassche, jmoyer, Alasdair G Kergon
In-Reply-To: <alpine.LRH.2.02.1512031039001.15134@file01.intranet.prod.int.rdu2.redhat.com>
On 3 December 2015 at 23:47, Mikulas Patocka <mpatocka@redhat.com> wrote:
>
>
> On Thu, 3 Dec 2015, Baolin Wang wrote:
>
>> On 3 December 2015 at 03:56, Alasdair G Kergon <agk@redhat.com> wrote:
>> > On Wed, Dec 02, 2015 at 08:46:54PM +0800, Baolin Wang wrote:
>> >> These are the benchmarks for request based dm-crypt. Please check it.
>> >
>> > Now please put request-based dm-crypt completely to one side and focus
>> > just on the existing bio-based code. Why is it slower and what can be
>> > adjusted to improve this?
>> >
>>
>> OK. I think I find something need to be point out.
>> 1. From the IO block size test in the performance report, for the
>> request based, we can find it can not get the corresponding
>> performance if we just expand the IO size. Because In dm crypt, it
>> will map the data buffer of one request with scatterlists, and send
>> all scatterlists of one request to the encryption engine to encrypt or
>> decrypt. I found if the scatterlist list number is small and each
>> scatterlist length is bigger, it will improve the encryption speed,
>
> This optimization is only applicable to XTS mode. XTS has its weaknesses
> and it is not recommended for encryption of more than 1TB of data
> ( http://grouper.ieee.org/groups/1619/email/msg02357.html )
>
> You can optimize bio-based dm-crypt as well (use larger encryption chunk
> than 512 bytes when the mode is XTS).
>
> The most commonly used mode aes-cbc-essiv:sha256 can't be optimized that
> way. You have to do encryption and decryption sector by sector because
> every sector has different IV.
Make sense. We'll optimize bio-based dm-crypt for XTS mode, and do
some investigations for none XTS mode.
>
> Mikulas
>
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: RAID 5,6 sequential writing seems slower in newer kernels
From: Weedy @ 2015-12-03 22:21 UTC (permalink / raw)
To: Phil Turmel; +Cc: Robert Kierski, Dallas Clement, linux-raid@vger.kernel.org
In-Reply-To: <566059E8.60804@turmel.org>
This is a bit off topic for this thread but is this tuning info on one
of the wiki's.
I mean tuning all the caches and whether you should have bitmaps on or off.
I'm going to build a new raid 5 (possibly raid 6) soon and would like
to saturate gigabit line.
I'd like to get my caches and stripes setup right for a 4 or 6 disk array.
^ permalink raw reply
* Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt
From: Mikulas Patocka @ 2015-12-03 15:49 UTC (permalink / raw)
To: Baolin Wang
Cc: Mark Brown, Jens Axboe, keith.busch, Jan Kara, Arnd Bergmann,
Mike Snitzer, neilb, LKML, linux-raid, dm-devel, Garg, Dinesh, tj,
bart.vanassche, jmoyer, Alasdair G Kergon
In-Reply-To: <CAMz4ku+5HpOJfqTd0C0i0_SMODFZw38dKqwK15Db4U37VNDVeg@mail.gmail.com>
On Thu, 3 Dec 2015, Baolin Wang wrote:
> On 3 December 2015 at 10:56, Baolin Wang <baolin.wang@linaro.org> wrote:
> > On 3 December 2015 at 03:56, Alasdair G Kergon <agk@redhat.com> wrote:
> >> On Wed, Dec 02, 2015 at 08:46:54PM +0800, Baolin Wang wrote:
> >>> These are the benchmarks for request based dm-crypt. Please check it.
> >>
> >> Now please put request-based dm-crypt completely to one side and focus
> >> just on the existing bio-based code. Why is it slower and what can be
> >> adjusted to improve this?
> >>
> >
> > OK. I think I find something need to be point out.
> > 1. From the IO block size test in the performance report, for the
> > request based, we can find it can not get the corresponding
> > performance if we just expand the IO size. Because In dm crypt, it
> > will map the data buffer of one request with scatterlists, and send
> > all scatterlists of one request to the encryption engine to encrypt or
> > decrypt. I found if the scatterlist list number is small and each
> > scatterlist length is bigger, it will improve the encryption speed,
> > that helps the engine palys best performance. But a big IO size does
> > not mean bigger scatterlists (maybe many scatterlists with small
> > length), that's why we can not get the corresponding performance if we
> > just expand the IO size I think.
> >
> > 2. Why bio based is slower?
> > If you understand 1, you can obviously understand the crypto engine
> > likes bigger scatterlists to improve the performance. But for bio
> > based, it only send one scatterlist (the scatterlist's length is
> > always '1 << SECTOR_SHIFT' = 512) to the crypto engine at one time. It
> > means if the bio size is 1M, the bio based will send 2048 times (evey
> > time the only one scatterlist length is 512 bytes) to crypto engine to
> > handle, which is more time-consuming and ineffective for the crypto
> > engine. But for request based, it can map the whole request with many
> > scatterlists (not just one scatterlist), and send all the scatterlists
> > to the crypto engine which can improve the performance, is it right?
> >
> > Another optimization solution I think is we can expand the scatterlist
> > entry number for bio based.
> >
>
> I did some testing about my assumption of expanding the scatterlist
> entry number for bio based. I did some modification for the bio based
> to support multiple scatterlists, then it will get the same
> performance as the request based things.
>
> 1. bio based with expanding the scatterlist entry
> time dd if=/dev/dm-0 of=/dev/null bs=64K count=16384 iflag=direct
> 1073741824 bytes (1.1 GB) copied, 94.5458 s, 11.4 MB/s
> real 1m34.562s
> user 0m0.030s
> sys 0m3.850s
>
> 2. Sequential read 1G with requset based:
> time dd if=/dev/dm-0 of=/dev/null bs=64K count=16384 iflag=direct
> 1073741824 bytes (1.1 GB) copied, 94.8922 s, 11.3 MB/s
> real 1m34.908s
> user 0m0.030s
> sys 0m4.000s
Measuring the system time this way is completely wrong because it doesn't
account for the time spent in kernel threads.
Mikulas
^ permalink raw reply
* Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt
From: Mikulas Patocka @ 2015-12-03 15:47 UTC (permalink / raw)
To: Baolin Wang
Cc: Mark Brown, Jens Axboe, keith.busch, Jan Kara, Arnd Bergmann,
Mike Snitzer, neilb, LKML, linux-raid, dm-devel, Garg, Dinesh, tj,
bart.vanassche, jmoyer, Alasdair G Kergon
In-Reply-To: <CAMz4kuL3qE-jE14F6ovAMRv6D-UFCdtiyUyi4ZDo+Lkr7_nA0Q@mail.gmail.com>
On Thu, 3 Dec 2015, Baolin Wang wrote:
> On 3 December 2015 at 03:56, Alasdair G Kergon <agk@redhat.com> wrote:
> > On Wed, Dec 02, 2015 at 08:46:54PM +0800, Baolin Wang wrote:
> >> These are the benchmarks for request based dm-crypt. Please check it.
> >
> > Now please put request-based dm-crypt completely to one side and focus
> > just on the existing bio-based code. Why is it slower and what can be
> > adjusted to improve this?
> >
>
> OK. I think I find something need to be point out.
> 1. From the IO block size test in the performance report, for the
> request based, we can find it can not get the corresponding
> performance if we just expand the IO size. Because In dm crypt, it
> will map the data buffer of one request with scatterlists, and send
> all scatterlists of one request to the encryption engine to encrypt or
> decrypt. I found if the scatterlist list number is small and each
> scatterlist length is bigger, it will improve the encryption speed,
This optimization is only applicable to XTS mode. XTS has its weaknesses
and it is not recommended for encryption of more than 1TB of data
( http://grouper.ieee.org/groups/1619/email/msg02357.html )
You can optimize bio-based dm-crypt as well (use larger encryption chunk
than 512 bytes when the mode is XTS).
The most commonly used mode aes-cbc-essiv:sha256 can't be optimized that
way. You have to do encryption and decryption sector by sector because
every sector has different IV.
Mikulas
> that helps the engine palys best performance. But a big IO size does
> not mean bigger scatterlists (maybe many scatterlists with small
> length), that's why we can not get the corresponding performance if we
> just expand the IO size I think.
>
> 2. Why bio based is slower?
> If you understand 1, you can obviously understand the crypto engine
> likes bigger scatterlists to improve the performance. But for bio
> based, it only send one scatterlist (the scatterlist's length is
> always '1 << SECTOR_SHIFT' = 512) to the crypto engine at one time. It
> means if the bio size is 1M, the bio based will send 2048 times (evey
> time the only one scatterlist length is 512 bytes) to crypto engine to
> handle, which is more time-consuming and ineffective for the crypto
> engine. But for request based, it can map the whole request with many
> scatterlists (not just one scatterlist), and send all the scatterlists
> to the crypto engine which can improve the performance, is it right?
>
> Another optimization solution I think is we can expand the scatterlist
> entry number for bio based.
^ permalink raw reply
* Re: RAID 5,6 sequential writing seems slower in newer kernels
From: Phil Turmel @ 2015-12-03 15:04 UTC (permalink / raw)
To: Robert Kierski, Dallas Clement, linux-raid@vger.kernel.org
In-Reply-To: <F7761B9B1D11B64BBB666019E9378117FDDF77@CFWEX01.americas.cray.com>
On 12/03/2015 09:19 AM, Robert Kierski wrote:
> Phil,
>
> I have a variety of testing tools that I use to corroborate the results of the others. So... IOR, XDD, fio, iozone, (and dd when I need something simple). Each of those can be run with a variety of options that simulate what an FS will submit to the block layer without adding the complexity, overhead, and uncertainty that an FS brings to the table. I've run the same tools through an FS, and found that at the bottom end of things, I can configure those tools to do exactly what the FS does... only when I'm looking at the traces, I don't have to scan past 100K lines while the FS is dealing with inodes, privileges, and other meta data.
Ok. Please cite the tool when you give a performance number, please.
> But to more precisely answer your question... as an example, if I'm using dd, I give this command:
>
> dd if=/dev/zero of=/dev/md0 bs=1M oflag=direct
Why oflag=direct ? And what do you get without it?
> Where /dev/md0 is the raid device I've configured.
>
> I don't use bitmaps, I've configured my raid using "--bitmap=none" and confirmed that mdadmin sees that there is no bitmap. I don't have alignment issues as my ramdisk has 512byte sectors. If something is somehow aligning things off 512byte boundaries when doing 1m writes.... I would be surprised. Also... I verified that the data written to disk falls at the boundaries I'm expecting.
Ok. I wasn't concerned about sector size. I was concerned about writes
not filling complete stripes in a single IO. Writes to parity raid are
broken up into 4k blocks in the stripe cache for parity calculation.
Each block in that stripe is separated from its mates by the chunk size.
If you don't write to all of them before the state machine decides to
compute, the parity devices will be read to perform RMW cycles (or the
other data members will be read to recompute from scratch). Either way,
when the 4k blocks are then written from the stripe, they have to have a
chance to get merged again.
> I tried RAID0 and got performance that is similar to what I was expecting -- 38G/s doing the writes.
Yep, those 1M writes are broken into chunk-sized writes for each member
and submitted as is. Raid456 breaks those down further for parity
calculation.
So, you probably have found a bug in post-stripe merging. Possibly due
to the extreme low latency of a ramdisk. Possibly an O_DIRECT side
effect. There's been a lot of work on parity raid in the past couple
years, both fixing bugs and adding features.
Sounds like time to bisect to locate the patches that make step changes
in performance on your specific hardware.
Phil
^ permalink raw reply
* Re: RAID 5,6 sequential writing seems slower in newer kernels
From: Dallas Clement @ 2015-12-03 14:39 UTC (permalink / raw)
To: Robert Kierski; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <F7761B9B1D11B64BBB666019E9378117FDDF77@CFWEX01.americas.cray.com>
On Thu, Dec 3, 2015 at 8:19 AM, Robert Kierski <rkierski@cray.com> wrote:
> Phil,
>
> I have a variety of testing tools that I use to corroborate the results of the others. So... IOR, XDD, fio, iozone, (and dd when I need something simple). Each of those can be run with a variety of options that simulate what an FS will submit to the block layer without adding the complexity, overhead, and uncertainty that an FS brings to the table. I've run the same tools through an FS, and found that at the bottom end of things, I can configure those tools to do exactly what the FS does... only when I'm looking at the traces, I don't have to scan past 100K lines while the FS is dealing with inodes, privileges, and other meta data.
>
> But to more precisely answer your question... as an example, if I'm using dd, I give this command:
>
> dd if=/dev/zero of=/dev/md0 bs=1M oflag=direct
>
> Where /dev/md0 is the raid device I've configured.
>
> I don't use bitmaps, I've configured my raid using "--bitmap=none" and confirmed that mdadmin sees that there is no bitmap. I don't have alignment issues as my ramdisk has 512byte sectors. If something is somehow aligning things off 512byte boundaries when doing 1m writes.... I would be surprised. Also... I verified that the data written to disk falls at the boundaries I'm expecting.
>
> I tried RAID0 and got performance that is similar to what I was expecting -- 38G/s doing the writes.
>
> I tried the 4.1 kernel, and was able to get better performance. It was actually 2x the 3.18 performance... but the 3.18 performance is so bad that twice horrible is still horrible.
>
> Bob Kierski
> Senior Storage Performance Engineer
> Cray Inc.
> 380 Jackson Street
> Suite 210
> St. Paul, MN 55101
> Tele: 651-967-9590
> Fax: 651-605-9001
> Cell: 651-890-7461
>
> I have a variety of testing tools that I use to corroborate the results of the others.
Robert, can you summarize your results for RAID 0, 1, 5, 6? I'm
interested to see what you are actually getting compared to what you
expected particularly for RAID 5 & 6.
^ permalink raw reply
* Re: RAID 5,6 sequential writing seems slower in newer kernels
From: Phil Turmel @ 2015-12-03 14:37 UTC (permalink / raw)
To: Robert Kierski, Dallas Clement; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <F7761B9B1D11B64BBB666019E9378117FDDF2F@CFWEX01.americas.cray.com>
On 12/03/2015 08:43 AM, Robert Kierski wrote:
> This is why I use Direct-IO to the bare metal block device instead of going through the FS. Rather than discussing the real problem, we're off in the weed talking about whether the tests should be using O_SYNC and whether there is a problem introduced in the latest version of the FS.
It's not off in the weeds for Dallas, the OP.
> FS's and cache are very good at hiding the problems of those things below them and prevent you from exercising the code you're interested in debugging.
Yep, you seem to have a real problem.
Phil
^ permalink raw reply
* RE: RAID 5,6 sequential writing seems slower in newer kernels
From: Robert Kierski @ 2015-12-03 14:19 UTC (permalink / raw)
To: Dallas Clement, linux-raid@vger.kernel.org
In-Reply-To: <CAE9DZUQGeGFxWigXJr4+HoWCn7+TYj4QBECLce75_zugV14wZQ@mail.gmail.com>
Phil,
I have a variety of testing tools that I use to corroborate the results of the others. So... IOR, XDD, fio, iozone, (and dd when I need something simple). Each of those can be run with a variety of options that simulate what an FS will submit to the block layer without adding the complexity, overhead, and uncertainty that an FS brings to the table. I've run the same tools through an FS, and found that at the bottom end of things, I can configure those tools to do exactly what the FS does... only when I'm looking at the traces, I don't have to scan past 100K lines while the FS is dealing with inodes, privileges, and other meta data.
But to more precisely answer your question... as an example, if I'm using dd, I give this command:
dd if=/dev/zero of=/dev/md0 bs=1M oflag=direct
Where /dev/md0 is the raid device I've configured.
I don't use bitmaps, I've configured my raid using "--bitmap=none" and confirmed that mdadmin sees that there is no bitmap. I don't have alignment issues as my ramdisk has 512byte sectors. If something is somehow aligning things off 512byte boundaries when doing 1m writes.... I would be surprised. Also... I verified that the data written to disk falls at the boundaries I'm expecting.
I tried RAID0 and got performance that is similar to what I was expecting -- 38G/s doing the writes.
I tried the 4.1 kernel, and was able to get better performance. It was actually 2x the 3.18 performance... but the 3.18 performance is so bad that twice horrible is still horrible.
Bob Kierski
Senior Storage Performance Engineer
Cray Inc.
380 Jackson Street
Suite 210
St. Paul, MN 55101
Tele: 651-967-9590
Fax: 651-605-9001
Cell: 651-890-7461
^ permalink raw reply
* RE: RAID 5,6 sequential writing seems slower in newer kernels
From: Robert Kierski @ 2015-12-03 13:43 UTC (permalink / raw)
To: Phil Turmel, Dallas Clement; +Cc: linux-raid@vger.kernel.org
In-Reply-To: <565FC548.1060302@turmel.org>
This is why I use Direct-IO to the bare metal block device instead of going through the FS. Rather than discussing the real problem, we're off in the weed talking about whether the tests should be using O_SYNC and whether there is a problem introduced in the latest version of the FS.
FS's and cache are very good at hiding the problems of those things below them and prevent you from exercising the code you're interested in debugging.
Bob Kierski
Senior Storage Performance Engineer
Cray Inc.
380 Jackson Street
Suite 210
St. Paul, MN 55101
Tele: 651-967-9590
Fax: 651-605-9001
Cell: 651-890-7461
^ permalink raw reply
* Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-12-03 11:27 UTC (permalink / raw)
To: Zdenek Kabelac
Cc: Mark Brown, Jens Axboe, keith.busch, Jan Kara, Arnd Bergmann,
Mike Snitzer, neilb, LKML, linux-raid, dm-devel, Garg, Dinesh, tj,
bart.vanassche, jmoyer, Alasdair G Kergon, Mikulas Patocka
In-Reply-To: <5660225E.30808@redhat.com>
On 3 December 2015 at 19:07, Zdenek Kabelac <zkabelac@redhat.com> wrote:
> Dne 3.12.2015 v 11:36 Baolin Wang napsal(a):
>
>> On 3 December 2015 at 10:56, Baolin Wang <baolin.wang@linaro.org> wrote:
>>>
>>> On 3 December 2015 at 03:56, Alasdair G Kergon <agk@redhat.com> wrote:
>>>>
>>>> On Wed, Dec 02, 2015 at 08:46:54PM +0800, Baolin Wang wrote:
>>>>>
>>>>> These are the benchmarks for request based dm-crypt. Please check it.
>>>>
>>>>
>>>> Now please put request-based dm-crypt completely to one side and focus
>>>> just on the existing bio-based code. Why is it slower and what can be
>>>> adjusted to improve this?
>>>>
>>>
>>> OK. I think I find something need to be point out.
>>> 1. From the IO block size test in the performance report, for the
>>> request based, we can find it can not get the corresponding
>>> performance if we just expand the IO size. Because In dm crypt, it
>>> will map the data buffer of one request with scatterlists, and send
>>> all scatterlists of one request to the encryption engine to encrypt or
>>> decrypt. I found if the scatterlist list number is small and each
>>> scatterlist length is bigger, it will improve the encryption speed,
>>> that helps the engine palys best performance. But a big IO size does
>>> not mean bigger scatterlists (maybe many scatterlists with small
>>> length), that's why we can not get the corresponding performance if we
>>> just expand the IO size I think.
>>>
>>> 2. Why bio based is slower?
>>> If you understand 1, you can obviously understand the crypto engine
>>> likes bigger scatterlists to improve the performance. But for bio
>>> based, it only send one scatterlist (the scatterlist's length is
>>> always '1 << SECTOR_SHIFT' = 512) to the crypto engine at one time. It
>>> means if the bio size is 1M, the bio based will send 2048 times (evey
>>> time the only one scatterlist length is 512 bytes) to crypto engine to
>>> handle, which is more time-consuming and ineffective for the crypto
>>> engine. But for request based, it can map the whole request with many
>>> scatterlists (not just one scatterlist), and send all the scatterlists
>>> to the crypto engine which can improve the performance, is it right?
>>>
>>> Another optimization solution I think is we can expand the scatterlist
>>> entry number for bio based.
>>>
>>
>> I did some testing about my assumption of expanding the scatterlist
>> entry number for bio based. I did some modification for the bio based
>> to support multiple scatterlists, then it will get the same
>> performance as the request based things.
>>
>> 1. bio based with expanding the scatterlist entry
>> time dd if=/dev/dm-0 of=/dev/null bs=64K count=16384 iflag=direct
>> 1073741824 bytes (1.1 GB) copied, 94.5458 s, 11.4 MB/s
>> real 1m34.562s
>> user 0m0.030s
>> sys 0m3.850s
>>
>> 2. Sequential read 1G with requset based:
>> time dd if=/dev/dm-0 of=/dev/null bs=64K count=16384 iflag=direct
>> 1073741824 bytes (1.1 GB) copied, 94.8922 s, 11.3 MB/s
>> real 1m34.908s
>> user 0m0.030s
>> sys 0m4.000s
>>
>> From the data, we can find the bio based also can get the same
>> performance as the request based. So if someone still don't like the
>> request based things, I think we can optimize the bio based by
>> expanding the scatterlists number. Thanks.
>>
>
>
> Hi
>
> Do you see any performance impact if you use with cryptsetup options:
>
> --perf-same_cpu_crypt
> --perf-submit_from_crypt_cpus
>
> with your regular unpatched kernel.
I did not see the performance impact with these options you said,
cause my board only has one cpu.
>
> Zdenek
>
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt
From: Zdenek Kabelac @ 2015-12-03 11:07 UTC (permalink / raw)
To: Baolin Wang, Mark Brown, Jens Axboe, keith.busch, Jan Kara,
Arnd Bergmann, Mike Snitzer, neilb, LKML, linux-raid, dm-devel,
Garg, Dinesh, tj, bart.vanassche, jmoyer, Alasdair G Kergon,
Mikulas Patocka
In-Reply-To: <CAMz4ku+5HpOJfqTd0C0i0_SMODFZw38dKqwK15Db4U37VNDVeg@mail.gmail.com>
Dne 3.12.2015 v 11:36 Baolin Wang napsal(a):
> On 3 December 2015 at 10:56, Baolin Wang <baolin.wang@linaro.org> wrote:
>> On 3 December 2015 at 03:56, Alasdair G Kergon <agk@redhat.com> wrote:
>>> On Wed, Dec 02, 2015 at 08:46:54PM +0800, Baolin Wang wrote:
>>>> These are the benchmarks for request based dm-crypt. Please check it.
>>>
>>> Now please put request-based dm-crypt completely to one side and focus
>>> just on the existing bio-based code. Why is it slower and what can be
>>> adjusted to improve this?
>>>
>>
>> OK. I think I find something need to be point out.
>> 1. From the IO block size test in the performance report, for the
>> request based, we can find it can not get the corresponding
>> performance if we just expand the IO size. Because In dm crypt, it
>> will map the data buffer of one request with scatterlists, and send
>> all scatterlists of one request to the encryption engine to encrypt or
>> decrypt. I found if the scatterlist list number is small and each
>> scatterlist length is bigger, it will improve the encryption speed,
>> that helps the engine palys best performance. But a big IO size does
>> not mean bigger scatterlists (maybe many scatterlists with small
>> length), that's why we can not get the corresponding performance if we
>> just expand the IO size I think.
>>
>> 2. Why bio based is slower?
>> If you understand 1, you can obviously understand the crypto engine
>> likes bigger scatterlists to improve the performance. But for bio
>> based, it only send one scatterlist (the scatterlist's length is
>> always '1 << SECTOR_SHIFT' = 512) to the crypto engine at one time. It
>> means if the bio size is 1M, the bio based will send 2048 times (evey
>> time the only one scatterlist length is 512 bytes) to crypto engine to
>> handle, which is more time-consuming and ineffective for the crypto
>> engine. But for request based, it can map the whole request with many
>> scatterlists (not just one scatterlist), and send all the scatterlists
>> to the crypto engine which can improve the performance, is it right?
>>
>> Another optimization solution I think is we can expand the scatterlist
>> entry number for bio based.
>>
>
> I did some testing about my assumption of expanding the scatterlist
> entry number for bio based. I did some modification for the bio based
> to support multiple scatterlists, then it will get the same
> performance as the request based things.
>
> 1. bio based with expanding the scatterlist entry
> time dd if=/dev/dm-0 of=/dev/null bs=64K count=16384 iflag=direct
> 1073741824 bytes (1.1 GB) copied, 94.5458 s, 11.4 MB/s
> real 1m34.562s
> user 0m0.030s
> sys 0m3.850s
>
> 2. Sequential read 1G with requset based:
> time dd if=/dev/dm-0 of=/dev/null bs=64K count=16384 iflag=direct
> 1073741824 bytes (1.1 GB) copied, 94.8922 s, 11.3 MB/s
> real 1m34.908s
> user 0m0.030s
> sys 0m4.000s
>
> From the data, we can find the bio based also can get the same
> performance as the request based. So if someone still don't like the
> request based things, I think we can optimize the bio based by
> expanding the scatterlists number. Thanks.
>
Hi
Do you see any performance impact if you use with cryptsetup options:
--perf-same_cpu_crypt
--perf-submit_from_crypt_cpus
with your regular unpatched kernel.
Zdenek
^ permalink raw reply
* Reshape stalls and fails when SELinux is enabled
From: Enrico Tagliavini @ 2015-12-03 11:04 UTC (permalink / raw)
To: linux-raid
[-- Attachment #1: Type: text/plain, Size: 3820 bytes --]
Hi everybody,
yesterday I tested a conversion of a 2 disk raid1 to a 4 disk raid5.
The reshape never completed, I rebooted the VM and the raid device was
unrecoverable. Backup-file was not useful unfortunately. Take a
comfortable seat, this is a bit long story, sorry :).
Environment:
- VM running on my laptop (Fedora 22), created with virt-manager,
qemu based. All disk images are qcow2 virtio devices.
- VM OS: centos 7 64 bit, fully updated as of today.
- mdadm version: 3.3.2-2.el7_1.1
- kernel version: 3.10.0-229.20.1.el7
- selinux policy targeted: 3.13.1-23.el7_1.21
How to reproduce:
* Attach four 1 GB disks to the VM, in the following example they are
vd[b,c,d,e].
* create a raid1 device with two disks: mdadm --create /dev/md0 -e
1.2 -n 2 -l raid1 -N raidtest /dev/vdb /dev/vdc
* [optional?]: I created a FS on it: mkfs.xfs -L test /dev/md0
* [optional?]: I created three files on it and noted the checksum, to
check for eventual corruptions
- mount /dev/md0 /mnt/raidtest/ && cd /mnt/raidtest/
- echo uno > first.txt ; echo due > second.txt ; touch third.txt
; shred -s 512M -f third.txt
- for file in *; do sha1sum $file >> sha1sum.txt; done
* check /proc/mdstat to make sure the raid1 sync operation is
complete (should really be fast but better safe than sorry)
* unmount /mnt/raidtest/
* add the two additional disks as spares:
- mdadm --manage /dev/md0 --add-spare /dev/vdd
- mdadm --manage /dev/md0 --add-spare /dev/vde
* grow and reshape: mdadm --grow /dev/md0 -n 4 --level=5
--backup-file /root/backup-md0 # (I also tried with
/var/local/backup-md0)
At this point an AVC will happen. Even if I'm in an interactive
session and, as such SELinux should not limit it normally, mdadm
process switches to mdadm_t type (maybe a forked process with its own
session group?) and is not allowed to write a file in the /root/ or
/var/ folders. This is ok, however mdadm keeps going instead of
aborting the reshape. It's running without a backup file, that's not
what the admin asked for since the --backup-file option is specified.
But even worst than this is that my reshape got stuck and never
completed. I waited a couple of hours but it remained at 0%. Something
was actually written to the backup file (which is weird given the AVC,
but it can be the original mdadm process not running under mdadm_t).
At this point I was kind of curious to test what would happen if a
distracted admin like me wont notice the problem and, days later,
would reboot the server due to security updates or anything else. The
result is an unrecoverable md array. I tried to assemble it back with
the backup file
mdadm --assemble /dev/md0 -u 6f53ec3e:d9868fef:12d3e243:8489561b
--backup-file /root/backup-md
But no way
mdadm: [sorry I copied wrong and the device name was lost] has an
active reshape - checking if critical section needs to be restored
mdadm: Failed to find backup of critical section
mdadm: Failed to restore critical section for reshape, sorry.
I retried the entire procedure from scratch, but this time with before
mdadm --grow I set SELinux in permissive mode with setenforce 0.
Everything was butter smooth this time. Reshape was almost instant for
such a small array, data was checksumming correctly and my array was
level 5.
Now there might be a problem with the SELinux policy here, but
honestly I think mdadm should just abort, whatever the reason of the
problem was. There might be other scenarios not involving SELinux
causing the same problem.
It would also be nice to suggest the user, if SELinux is active, to
change the context of the backup file to something SELinux will permit
(mdadm_map_t, mdadm_var_run_t?).
Attached you can also find the AVC denial for my entire day of testing.
Thank you for your help.
Best regards.
Enrico Tagliavini
[-- Attachment #2: changeraid1_to_raid5.txt --]
[-- Type: text/plain, Size: 68 bytes --]
mdadm --grow /dev/md0 -n 4 --level=5 --backup-file=/root/backup-md0
^ 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