* Re: [PATCH RESEND v2 17/18] fuse: Restrict allow_other to the superblock's namespace or a descendant
From: Miklos Szeredi @ 2016-03-09 11:40 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn,
Richard Weinberger, Austin S Hemmelgarn, linux-kernel,
linux-bcache, dm-devel, linux-raid, linux-mtd, linux-fsdevel,
fuse-devel, linux-security-module, selinux
In-Reply-To: <1451930639-94331-18-git-send-email-seth.forshee@canonical.com>
On Mon, Jan 04, 2016 at 12:03:56PM -0600, Seth Forshee wrote:
> Unprivileged users are normally restricted from mounting with the
> allow_other option by system policy, but this could be bypassed
> for a mount done with user namespace root permissions. In such
> cases allow_other should not allow users outside the userns
> to access the mount as doing so would give the unprivileged user
> the ability to manipulate processes it would otherwise be unable
> to manipulate. Restrict allow_other to apply to users in the same
> userns used at mount or a descendant of that namespace.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/fuse/dir.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 8fd9fe4dcd43..24e4cdb554f1 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1015,7 +1015,7 @@ int fuse_allow_current_process(struct fuse_conn *fc)
> const struct cred *cred;
>
> if (fc->flags & FUSE_ALLOW_OTHER)
> - return 1;
> + return current_in_userns(fc->user_ns);
>
> cred = current_cred();
> if (uid_eq(cred->euid, fc->user_id) &&
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH RESEND v2 16/18] fuse: Support fuse filesystems outside of init_user_ns
From: Miklos Szeredi @ 2016-03-09 11:29 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn,
Richard Weinberger, Austin S Hemmelgarn, linux-kernel,
linux-bcache, dm-devel, linux-raid, linux-mtd, linux-fsdevel,
fuse-devel, linux-security-module, selinux
In-Reply-To: <1451930639-94331-17-git-send-email-seth.forshee@canonical.com>
On Mon, Jan 04, 2016 at 12:03:55PM -0600, Seth Forshee wrote:
> In order to support mounts from namespaces other than
> init_user_ns, fuse must translate uids and gids to/from the
> userns of the process servicing requests on /dev/fuse. This
> patch does that, with a couple of restrictions on the namespace:
>
> - The userns for the fuse connection is fixed to the namespace
> from which /dev/fuse is opened.
>
> - The namespace must be the same as s_user_ns.
>
> These restrictions simplify the implementation by avoiding the
> need to pass around userns references and by allowing fuse to
> rely on the checks in inode_change_ok for ownership changes.
> Either restriction could be relaxed in the future if needed.
>
> For cuse the namespace used for the connection is also simply
> current_user_ns() at the time /dev/cuse is opened.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
> fs/fuse/cuse.c | 3 ++-
> fs/fuse/dev.c | 13 ++++++++-----
> fs/fuse/dir.c | 14 +++++++-------
> fs/fuse/fuse_i.h | 6 +++++-
> fs/fuse/inode.c | 35 +++++++++++++++++++++++------------
> 5 files changed, 45 insertions(+), 26 deletions(-)
>
> diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
> index eae2c11268bc..a10aca57bfe4 100644
> --- a/fs/fuse/cuse.c
> +++ b/fs/fuse/cuse.c
> @@ -48,6 +48,7 @@
> #include <linux/stat.h>
> #include <linux/module.h>
> #include <linux/uio.h>
> +#include <linux/user_namespace.h>
>
> #include "fuse_i.h"
>
> @@ -498,7 +499,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
> if (!cc)
> return -ENOMEM;
>
> - fuse_conn_init(&cc->fc);
> + fuse_conn_init(&cc->fc, current_user_ns());
>
> fud = fuse_dev_alloc(&cc->fc);
> if (!fud) {
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index a4f6f30d6d86..11b4cb0a0e2f 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -127,8 +127,8 @@ static void __fuse_put_request(struct fuse_req *req)
>
> static void fuse_req_init_context(struct fuse_conn *fc, struct fuse_req *req)
> {
> - req->in.h.uid = from_kuid_munged(&init_user_ns, current_fsuid());
> - req->in.h.gid = from_kgid_munged(&init_user_ns, current_fsgid());
> + req->in.h.uid = from_kuid(fc->user_ns, current_fsuid());
> + req->in.h.gid = from_kgid(fc->user_ns, current_fsgid());
> req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
> }
>
> @@ -186,7 +186,8 @@ static struct fuse_req *__fuse_get_req(struct fuse_conn *fc, unsigned npages,
> __set_bit(FR_WAITING, &req->flags);
> if (for_background)
> __set_bit(FR_BACKGROUND, &req->flags);
> - if (req->in.h.pid == 0) {
> + if (req->in.h.pid == 0 || req->in.h.uid == (uid_t)-1 ||
> + req->in.h.gid == (gid_t)-1) {
> fuse_put_request(fc, req);
> return ERR_PTR(-EOVERFLOW);
> }
> @@ -1248,7 +1249,8 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
> struct fuse_in *in;
> unsigned reqsize;
>
> - if (task_active_pid_ns(current) != fc->pid_ns)
> + if (task_active_pid_ns(current) != fc->pid_ns ||
> + current_user_ns() != fc->user_ns)
> return -EIO;
>
> restart:
> @@ -1880,7 +1882,8 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud,
> struct fuse_req *req;
> struct fuse_out_header oh;
>
> - if (task_active_pid_ns(current) != fc->pid_ns)
> + if (task_active_pid_ns(current) != fc->pid_ns ||
> + current_user_ns() != fc->user_ns)
> return -EIO;
>
> if (nbytes < sizeof(struct fuse_out_header))
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 5e2e08712d3b..8fd9fe4dcd43 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -841,8 +841,8 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
> stat->ino = attr->ino;
> stat->mode = (inode->i_mode & S_IFMT) | (attr->mode & 07777);
> stat->nlink = attr->nlink;
> - stat->uid = make_kuid(&init_user_ns, attr->uid);
> - stat->gid = make_kgid(&init_user_ns, attr->gid);
> + stat->uid = inode->i_uid;
> + stat->gid = inode->i_gid;
This breaks the attr_version logic in fuse_change_attributes().
So just use make_k[ug]id() here as well.
> stat->rdev = inode->i_rdev;
> stat->atime.tv_sec = attr->atime;
> stat->atime.tv_nsec = attr->atimensec;
> @@ -1455,17 +1455,17 @@ static bool update_mtime(unsigned ivalid, bool trust_local_mtime)
> return true;
> }
>
> -static void iattr_to_fattr(struct iattr *iattr, struct fuse_setattr_in *arg,
> - bool trust_local_cmtime)
> +static void iattr_to_fattr(struct fuse_conn *fc, struct iattr *iattr,
> + struct fuse_setattr_in *arg, bool trust_local_cmtime)
> {
> unsigned ivalid = iattr->ia_valid;
>
> if (ivalid & ATTR_MODE)
> arg->valid |= FATTR_MODE, arg->mode = iattr->ia_mode;
> if (ivalid & ATTR_UID)
> - arg->valid |= FATTR_UID, arg->uid = from_kuid(&init_user_ns, iattr->ia_uid);
> + arg->valid |= FATTR_UID, arg->uid = from_kuid(fc->user_ns, iattr->ia_uid);
> if (ivalid & ATTR_GID)
> - arg->valid |= FATTR_GID, arg->gid = from_kgid(&init_user_ns, iattr->ia_gid);
> + arg->valid |= FATTR_GID, arg->gid = from_kgid(fc->user_ns, iattr->ia_gid);
> if (ivalid & ATTR_SIZE)
> arg->valid |= FATTR_SIZE, arg->size = iattr->ia_size;
> if (ivalid & ATTR_ATIME) {
> @@ -1625,7 +1625,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
>
> memset(&inarg, 0, sizeof(inarg));
> memset(&outarg, 0, sizeof(outarg));
> - iattr_to_fattr(attr, &inarg, trust_local_cmtime);
> + iattr_to_fattr(fc, attr, &inarg, trust_local_cmtime);
> if (file) {
> struct fuse_file *ff = file->private_data;
> inarg.valid |= FATTR_FH;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 143b595197b6..5897805405ba 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -23,6 +23,7 @@
> #include <linux/poll.h>
> #include <linux/workqueue.h>
> #include <linux/pid_namespace.h>
> +#include <linux/user_namespace.h>
>
> /** Max number of pages that can be used in a single read request */
> #define FUSE_MAX_PAGES_PER_REQ 32
> @@ -460,6 +461,9 @@ struct fuse_conn {
> /** The pid namespace for this mount */
> struct pid_namespace *pid_ns;
>
> + /** The user namespace for this mount */
> + struct user_namespace *user_ns;
> +
> /** The fuse mount flags for this mount */
> unsigned flags;
>
> @@ -855,7 +859,7 @@ struct fuse_conn *fuse_conn_get(struct fuse_conn *fc);
> /**
> * Initialize fuse_conn
> */
> -void fuse_conn_init(struct fuse_conn *fc);
> +void fuse_conn_init(struct fuse_conn *fc, struct user_namespace *user_ns);
>
> /**
> * Release reference to fuse_conn
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 2f31874ea9db..b7bdfdac3521 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -167,8 +167,8 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
> inode->i_ino = fuse_squash_ino(attr->ino);
> inode->i_mode = (inode->i_mode & S_IFMT) | (attr->mode & 07777);
> set_nlink(inode, attr->nlink);
> - inode->i_uid = make_kuid(&init_user_ns, attr->uid);
> - inode->i_gid = make_kgid(&init_user_ns, attr->gid);
> + inode->i_uid = make_kuid(fc->user_ns, attr->uid);
> + inode->i_gid = make_kgid(fc->user_ns, attr->gid);
> inode->i_blocks = attr->blocks;
> inode->i_atime.tv_sec = attr->atime;
> inode->i_atime.tv_nsec = attr->atimensec;
> @@ -467,12 +467,15 @@ static int fuse_match_uint(substring_t *s, unsigned int *res)
> return err;
> }
>
> -static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev)
> +static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev,
> + struct user_namespace *user_ns)
> {
> char *p;
> memset(d, 0, sizeof(struct fuse_mount_data));
> d->max_read = ~0;
> d->blksize = FUSE_DEFAULT_BLKSIZE;
> + d->user_id = make_kuid(user_ns, 0);
> + d->group_id = make_kgid(user_ns, 0);
It is true that if "user_id=" or "group_id" options were omitted we used the
zero uid/gid values. However, this isn't actually used by anybody AFAIK, and
generalizing it for userns doesn't seem to make much sense.
So I suggest we that we instead return an error if mounting from a userns AND
neither "allow_other" nor both "user_id" and "group_id" are specified.
>
> while ((p = strsep(&opt, ",")) != NULL) {
> int token;
> @@ -503,7 +506,7 @@ static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev)
> case OPT_USER_ID:
> if (fuse_match_uint(&args[0], &uv))
> return 0;
> - d->user_id = make_kuid(current_user_ns(), uv);
> + d->user_id = make_kuid(user_ns, uv);
> if (!uid_valid(d->user_id))
> return 0;
> d->user_id_present = 1;
> @@ -512,7 +515,7 @@ static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev)
> case OPT_GROUP_ID:
> if (fuse_match_uint(&args[0], &uv))
> return 0;
> - d->group_id = make_kgid(current_user_ns(), uv);
> + d->group_id = make_kgid(user_ns, uv);
> if (!gid_valid(d->group_id))
> return 0;
> d->group_id_present = 1;
> @@ -555,8 +558,10 @@ static int fuse_show_options(struct seq_file *m, struct dentry *root)
> struct super_block *sb = root->d_sb;
> struct fuse_conn *fc = get_fuse_conn_super(sb);
>
> - seq_printf(m, ",user_id=%u", from_kuid_munged(&init_user_ns, fc->user_id));
> - seq_printf(m, ",group_id=%u", from_kgid_munged(&init_user_ns, fc->group_id));
> + seq_printf(m, ",user_id=%u",
> + from_kuid_munged(fc->user_ns, fc->user_id));
> + seq_printf(m, ",group_id=%u",
> + from_kgid_munged(fc->user_ns, fc->group_id));
> if (fc->flags & FUSE_DEFAULT_PERMISSIONS)
> seq_puts(m, ",default_permissions");
> if (fc->flags & FUSE_ALLOW_OTHER)
> @@ -587,7 +592,7 @@ static void fuse_pqueue_init(struct fuse_pqueue *fpq)
> fpq->connected = 1;
> }
>
> -void fuse_conn_init(struct fuse_conn *fc)
> +void fuse_conn_init(struct fuse_conn *fc, struct user_namespace *user_ns)
> {
> memset(fc, 0, sizeof(*fc));
> spin_lock_init(&fc->lock);
> @@ -611,6 +616,7 @@ void fuse_conn_init(struct fuse_conn *fc)
> fc->attr_version = 1;
> get_random_bytes(&fc->scramble_key, sizeof(fc->scramble_key));
> fc->pid_ns = get_pid_ns(task_active_pid_ns(current));
> + fc->user_ns = get_user_ns(user_ns);
> }
> EXPORT_SYMBOL_GPL(fuse_conn_init);
>
> @@ -620,6 +626,7 @@ void fuse_conn_put(struct fuse_conn *fc)
> if (fc->destroy_req)
> fuse_request_free(fc->destroy_req);
> put_pid_ns(fc->pid_ns);
> + put_user_ns(fc->user_ns);
> fc->release(fc);
> }
> }
> @@ -1046,7 +1053,7 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
>
> sb->s_flags &= ~(MS_NOSEC | MS_I_VERSION);
>
> - if (!parse_fuse_opt(data, &d, is_bdev))
> + if (!parse_fuse_opt(data, &d, is_bdev, sb->s_user_ns))
> goto err;
>
> if (is_bdev) {
> @@ -1070,8 +1077,12 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
> if (!file)
> goto err;
>
> - if ((file->f_op != &fuse_dev_operations) ||
> - (file->f_cred->user_ns != &init_user_ns))
> + /*
> + * Require mount to happen from the same user namespace which
> + * opened /dev/fuse to prevent potential attacks.
> + */
> + if (file->f_op != &fuse_dev_operations ||
> + file->f_cred->user_ns != sb->s_user_ns)
> goto err_fput;
>
> fc = kmalloc(sizeof(*fc), GFP_KERNEL);
> @@ -1079,7 +1090,7 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
> if (!fc)
> goto err_fput;
>
> - fuse_conn_init(fc);
> + fuse_conn_init(fc, sb->s_user_ns);
> fc->release = fuse_free_conn;
>
> fud = fuse_dev_alloc(fc);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH RESEND v2 15/18] fuse: Add support for pid namespaces
From: Miklos Szeredi @ 2016-03-09 10:53 UTC (permalink / raw)
To: Seth Forshee
Cc: Eric W. Biederman, Alexander Viro, Serge Hallyn,
Richard Weinberger, Austin S Hemmelgarn, linux-kernel,
linux-bcache, dm-devel, linux-raid, linux-mtd, linux-fsdevel,
fuse-devel, linux-security-module, selinux, Miklos Szeredi
In-Reply-To: <1451930639-94331-16-git-send-email-seth.forshee@canonical.com>
On Mon, Jan 04, 2016 at 12:03:54PM -0600, Seth Forshee wrote:
> If the userspace process servicing fuse requests is running in
> a pid namespace then pids passed via the fuse fd need to be
> translated relative to that namespace. Capture the pid namespace
> in use when the filesystem is mounted and use this for pid
> translation.
>
> Since no use case currently exists for changing namespaces all
> translations are done relative to the pid namespace in use when
> /dev/fuse is opened.
The above doesn't match what the patch does.
- FUSE captures namespace at mount time
- CUSE captures namespace at /dev/cuse open
> Mounting or /dev/fuse IO from another
> namespace will return errors.
>
> Requests from processes whose pid cannot be translated into the
> target namespace are not permitted, except for requests
> allocated via fuse_get_req_nofail_nopages. For no-fail requests
> in.h.pid will be 0 if the pid translation fails.
>
> File locking changes based on previous work done by Eric
> Biederman.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Not sure how my SOB got on this patch, use this instead:
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/fuse/dev.c | 19 +++++++++++++++----
> fs/fuse/file.c | 22 +++++++++++++++++-----
> fs/fuse/fuse_i.h | 4 ++++
> fs/fuse/inode.c | 3 +++
> 4 files changed, 39 insertions(+), 9 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index ebb5e37455a0..a4f6f30d6d86 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -19,6 +19,7 @@
> #include <linux/pipe_fs_i.h>
> #include <linux/swap.h>
> #include <linux/splice.h>
> +#include <linux/sched.h>
>
> MODULE_ALIAS_MISCDEV(FUSE_MINOR);
> MODULE_ALIAS("devname:fuse");
> @@ -124,11 +125,11 @@ static void __fuse_put_request(struct fuse_req *req)
> atomic_dec(&req->count);
> }
>
> -static void fuse_req_init_context(struct fuse_req *req)
> +static void fuse_req_init_context(struct fuse_conn *fc, struct fuse_req *req)
> {
> req->in.h.uid = from_kuid_munged(&init_user_ns, current_fsuid());
> req->in.h.gid = from_kgid_munged(&init_user_ns, current_fsgid());
> - req->in.h.pid = current->pid;
> + req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
> }
>
> void fuse_set_initialized(struct fuse_conn *fc)
> @@ -181,10 +182,14 @@ static struct fuse_req *__fuse_get_req(struct fuse_conn *fc, unsigned npages,
> goto out;
> }
>
> - fuse_req_init_context(req);
> + fuse_req_init_context(fc, req);
> __set_bit(FR_WAITING, &req->flags);
> if (for_background)
> __set_bit(FR_BACKGROUND, &req->flags);
> + if (req->in.h.pid == 0) {
> + fuse_put_request(fc, req);
> + return ERR_PTR(-EOVERFLOW);
> + }
>
> return req;
>
> @@ -274,7 +279,7 @@ struct fuse_req *fuse_get_req_nofail_nopages(struct fuse_conn *fc,
> if (!req)
> req = get_reserved_req(fc, file);
>
> - fuse_req_init_context(req);
> + fuse_req_init_context(fc, req);
> __set_bit(FR_WAITING, &req->flags);
> __clear_bit(FR_BACKGROUND, &req->flags);
> return req;
> @@ -1243,6 +1248,9 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
> struct fuse_in *in;
> unsigned reqsize;
>
> + if (task_active_pid_ns(current) != fc->pid_ns)
> + return -EIO;
> +
> restart:
> spin_lock(&fiq->waitq.lock);
> err = -EAGAIN;
> @@ -1872,6 +1880,9 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud,
> struct fuse_req *req;
> struct fuse_out_header oh;
>
> + if (task_active_pid_ns(current) != fc->pid_ns)
> + return -EIO;
> +
> if (nbytes < sizeof(struct fuse_out_header))
> return -EINVAL;
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index e0faf8f2c868..a6c7484c94ee 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2061,7 +2061,8 @@ static int fuse_direct_mmap(struct file *file, struct vm_area_struct *vma)
> return generic_file_mmap(file, vma);
> }
>
> -static int convert_fuse_file_lock(const struct fuse_file_lock *ffl,
> +static int convert_fuse_file_lock(struct fuse_conn *fc,
> + const struct fuse_file_lock *ffl,
> struct file_lock *fl)
> {
> switch (ffl->type) {
> @@ -2076,7 +2077,14 @@ static int convert_fuse_file_lock(const struct fuse_file_lock *ffl,
>
> fl->fl_start = ffl->start;
> fl->fl_end = ffl->end;
> - fl->fl_pid = ffl->pid;
> +
> + /*
> + * Convert pid into the caller's pid namespace. If the pid
> + * does not map into the namespace fl_pid will get set to 0.
> + */
> + rcu_read_lock();
> + fl->fl_pid = pid_vnr(find_pid_ns(ffl->pid, fc->pid_ns));
> + rcu_read_unlock();
> break;
>
> default:
> @@ -2125,7 +2133,7 @@ static int fuse_getlk(struct file *file, struct file_lock *fl)
> args.out.args[0].value = &outarg;
> err = fuse_simple_request(fc, &args);
> if (!err)
> - err = convert_fuse_file_lock(&outarg.lk, fl);
> + err = convert_fuse_file_lock(fc, &outarg.lk, fl);
>
> return err;
> }
> @@ -2137,7 +2145,8 @@ static int fuse_setlk(struct file *file, struct file_lock *fl, int flock)
> FUSE_ARGS(args);
> struct fuse_lk_in inarg;
> int opcode = (fl->fl_flags & FL_SLEEP) ? FUSE_SETLKW : FUSE_SETLK;
> - pid_t pid = fl->fl_type != F_UNLCK ? current->tgid : 0;
> + struct pid *pid = fl->fl_type != F_UNLCK ? task_tgid(current) : NULL;
> + pid_t pid_nr = pid_nr_ns(pid, fc->pid_ns);
> int err;
>
> if (fl->fl_lmops && fl->fl_lmops->lm_grant) {
> @@ -2149,7 +2158,10 @@ static int fuse_setlk(struct file *file, struct file_lock *fl, int flock)
> if (fl->fl_flags & FL_CLOSE)
> return 0;
>
> - fuse_lk_fill(&args, file, fl, opcode, pid, flock, &inarg);
> + if (pid && pid_nr == 0)
> + return -EOVERFLOW;
> +
> + fuse_lk_fill(&args, file, fl, opcode, pid_nr, flock, &inarg);
> err = fuse_simple_request(fc, &args);
>
> /* locking is restartable */
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 405113101db8..143b595197b6 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -22,6 +22,7 @@
> #include <linux/rbtree.h>
> #include <linux/poll.h>
> #include <linux/workqueue.h>
> +#include <linux/pid_namespace.h>
>
> /** Max number of pages that can be used in a single read request */
> #define FUSE_MAX_PAGES_PER_REQ 32
> @@ -456,6 +457,9 @@ struct fuse_conn {
> /** The group id for this mount */
> kgid_t group_id;
>
> + /** The pid namespace for this mount */
> + struct pid_namespace *pid_ns;
> +
> /** The fuse mount flags for this mount */
> unsigned flags;
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 2913db2a5b99..2f31874ea9db 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -20,6 +20,7 @@
> #include <linux/random.h>
> #include <linux/sched.h>
> #include <linux/exportfs.h>
> +#include <linux/pid_namespace.h>
>
> MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
> MODULE_DESCRIPTION("Filesystem in Userspace");
> @@ -609,6 +610,7 @@ void fuse_conn_init(struct fuse_conn *fc)
> fc->connected = 1;
> fc->attr_version = 1;
> get_random_bytes(&fc->scramble_key, sizeof(fc->scramble_key));
> + fc->pid_ns = get_pid_ns(task_active_pid_ns(current));
> }
> EXPORT_SYMBOL_GPL(fuse_conn_init);
>
> @@ -617,6 +619,7 @@ void fuse_conn_put(struct fuse_conn *fc)
> if (atomic_dec_and_test(&fc->count)) {
> if (fc->destroy_req)
> fuse_request_free(fc->destroy_req);
> + put_pid_ns(fc->pid_ns);
> fc->release(fc);
> }
> }
> --
> 1.9.1
>
^ permalink raw reply
* Re: When do you replace old hard drives in a raid6?
From: Roman Mamedov @ 2016-03-09 6:59 UTC (permalink / raw)
To: Mikael Abrahamsson; +Cc: Linux Raid
In-Reply-To: <alpine.DEB.2.02.1603090733030.31096@uplift.swm.pp.se>
[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]
On Wed, 9 Mar 2016 07:43:16 +0100 (CET)
Mikael Abrahamsson <swmike@swm.pp.se> wrote:
> I had very high failure rates of the early 2TB WD Greens, but I still have
> some WD20EARS and WD20EADS that are alive after 58k hours.
In my experience a major cause for the WD failures is that they develop rust
on the PCB contacts which connect to the drive insides, see e.g.:
http://ods.com.ua/win/rus/other/hdd/2/wd_cont2.jpg
http://www.chipmaker.ru/uploads/monthly_12_2013/post/image/post-7336-020632100%201388236036.jpg
and: https://www.youtube.com/watch?v=tDTt_yjYYQ8
If such WD drive has just developed several unreadable sectors, this often can
be solved by checking and cleaning those contacts, then overwriting the whole
drive with zeroes (or well, with whatever you want, the point is to rewrite
all the "badly written" areas), then it will likely work fine for years after
that.
> One of the slightly lower power on time ones has a scary load cycle count
> though:
>
> Device Model: WDC WD20EARS-00S8B1
> 9 Power_On_Hours 0x0032 033 033 000 Old_age Always - 49255
> 193 Load_Cycle_Count 0x0032 001 001 000 Old_age Always - 1317839
This can be disabled:
http://www.storagereview.com/how_to_stop_excessive_load_cycles_on_the_western_digital_2tb_caviar_green_wd20ears_with_wdidle3
http://idle3-tools.sourceforge.net/
--
With respect,
Roman
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: When do you replace old hard drives in a raid6?
From: Mikael Abrahamsson @ 2016-03-09 6:43 UTC (permalink / raw)
To: Linux Raid
In-Reply-To: <56DF6A39.4020904@gmail.com>
On Tue, 8 Mar 2016, Ram Ramesh wrote:
> My disks have about 10K hours (my server only runs from 4pm-2am). I
> think I have quite a bit of life left assuming an on/off cycle is not as
> bad as extra 14 hours of run time.
I had very high failure rates of the early 2TB WD Greens, but I still have
some WD20EARS and WD20EADS that are alive after 58k hours.
One of the slightly lower power on time ones has a scary load cycle count
though:
Device Model: WDC WD20EARS-00S8B1
9 Power_On_Hours 0x0032 033 033 000 Old_age Always - 49255
193 Load_Cycle_Count 0x0032 001 001 000 Old_age Always - 1317839
I'm running this as RAID6+spare and I'm just going to let these run until
they fail and replace them with WD REDs one by one. I clearly had bathtub
effect where I had several drives I replaced under warranty in the first
1-2 years of their lifetime, but the ones that replaced them, and the ones
that didn't fail, still seems to be doing fine.
I have two drives with reallocated sectors, but it's 3 and 5 sectors
respectively, so this is not worrying yet.
I wish we had raid6e (or whatever to call it) with 3 parity drives, I'd
really like to run that instead of raid6+spare.
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* [PATCH] Fix regression during add devices
From: Coly Li @ 2016-03-09 5:20 UTC (permalink / raw)
To: linux-raid; +Cc: Hannes Reinecke, Coly Li, Neil Brown
From: Hannes Reinecke <hare@suse.de>
Commit d180d2aa2a17 ("Manage: fix test for 'is array failed'.")
introduced a regression which would not allow to re-add new
drivers to a failed array.
The patch is written by Hannes Reinecke, Neil Brown points out
the buggy commit ID is d180d2aa2a17. Coly helps to submit the
patch to mdadm upstream.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: Coly Li <colyli@suse.de>
Cc: Neil Brown <neilb@suse.com>
---
Manage.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Manage.c b/Manage.c
index 7e1b94b..073ddb9 100644
--- a/Manage.c
+++ b/Manage.c
@@ -879,10 +879,10 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
continue;
if (disc.major == 0 && disc.minor == 0)
continue;
- found++;
if (!(disc.state & (1<<MD_DISK_SYNC)))
continue;
avail[disc.raid_disk] = 1;
+ found++;
}
array_failed = !enough(array->level, array->raid_disks,
array->layout, 1, avail);
--
2.6.2
^ permalink raw reply related
* Re: Import/recover RAID6 created with SAS2108?
From: Dan Russell @ 2016-03-09 3:20 UTC (permalink / raw)
To: linux-raid
In-Reply-To: <20160307215026.GA8042@EIS>
Thank you. I’ve made some progress. My hardware controller uses a DDF container, which I’m able to start and inspect with mdadm. Based on the mdadm release notes, I’ve updated from v3.2.5 to v3.4.
The data is not encrypted, and I believe I have the HDD order and chunk sizes correct (in part because the DDF container matches my independently-gathered notes).
The HDD order is sdi, sdj, sdag, sdl - sdz, sdaa - sdaf (when the array initially failed, I replaced the drive in slot 2 with a new drive and started a rebuild. The partially-rebuilt drive is sdk, the original “failed” drive is sdag).
When I did an Incremental on the container, I ended up with 3 inactive arrays (one 2-disk, one 20-disk, one 2-disk), which is comparable to what the hardware RAID controller told me about foreign configurations. So I tried to create a new 24-disk array with the same parameters as the old.
I am able to create an array and see the LVM label on md1. However fdisk and mdadm are reporting the array is 17.6TB in size, whereas it should be 66TB (24 3TB HDDs RAID6). This is the same whether I specify or leave off the —size option when creating the array.
The first mdadm —create to make the ddf container shows that the ctime for sdag and sdp is Jan 17; this is the last time I booted this server prior to the breakage. I’m wondering if there’s some way I can use the container metadata from either of those drives and ignore the rest?
Note that md0 is my root volume and is perfectly fine; it is only listed because I didn’t want to edit any command output.
# uname -r
3.13.0-49-generic
# mdadm -V
mdadm - v3.4 - 28th January 2016
# cat /proc/mdstat
Personalities : [linear] [multipath] [raid0] [raid1] [raid6] [raid5] [raid4] [raid10]
md0 : active raid1 sdh1[1] sdg1[0]
124967808 blocks super 1.2 [2/2] [UU]
# mdadm --create /dev/md127 -e ddf -l container -n 24 /dev/mapper/sdi /dev/mapper/sdj /dev/mapper/sdag /dev/mapper/sdl /dev/mapper/sdm /dev/mapper/sdn /dev/mapper/sdo /dev/mapper/sdp /dev/mapper/sdq /dev/mapper/sdr /dev/mapper/sds /dev/mapper/sdt /dev/mapper/sdu /dev/mapper/sdv /dev/mapper/sdw /dev/mapper/sdx /dev/mapper/sdy /dev/mapper/sdz /dev/mapper/sdaa /dev/mapper/sdab /dev/mapper/sdac /dev/mapper/sdad /dev/mapper/sdae /dev/mapper/sdaf
mdadm: /dev/mapper/sdj appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdag appears to be part of a raid array:
level=container devices=212 ctime=Sun Jan 17 17:03:35 2016
mdadm: /dev/mapper/sdl appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdm appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdn appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdo appears to be part of a raid array:
level=container devices=199 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdp appears to be part of a raid array:
level=container devices=212 ctime=Sun Jan 17 17:03:35 2016
mdadm: /dev/mapper/sdq appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdr appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sds appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdt appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdu appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdv appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdw appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdx appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdy appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdz appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdaa appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdab appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdac appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdad appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdae appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
mdadm: /dev/mapper/sdaf appears to be part of a raid array:
level=container devices=198 ctime=Fri Feb 26 13:38:07 2016
Continue creating array? y
mdadm: container /dev/md127 prepared.
# cat /proc/mdstat
Personalities : [linear] [multipath] [raid0] [raid1] [raid6] [raid5] [raid4] [raid10]
md127 : inactive dm-24[23](S) dm-23[22](S) dm-22[21](S) dm-21[20](S) dm-20[19](S) dm-19[18](S) dm-18[17](S) dm-17[16](S) dm-16[15](S) dm-15[14](S) dm-14[13](S) dm-13[12](S) dm-12[11](S) dm-11[10](S) dm-10[9](S) dm-9[8](S) dm-8[7](S) dm-7[6](S) dm-6[5](S) dm-5[4](S) dm-4[3](S) dm-25[2](S) dm-2[1](S) dm-1[0](S)
786432 blocks super external:ddf
md0 : active raid1 sdh1[1] sdg1[0]
124967808 blocks super 1.2 [2/2] [UU]
# mdadm --create /dev/md1 --assume-clean --level=6 --raid-devices=24 --chunk=64 --size=2929686528 /dev/mapper/sdi /dev/mapper/sdj /dev/mapper/sdag /dev/mapper/sdl /dev/mapper/sdm /dev/mapper/sdn /dev/mapper/sdo /dev/mapper/sdp /dev/mapper/sdq /dev/mapper/sdr /dev/mapper/sds /dev/mapper/sdt /dev/mapper/sdu /dev/mapper/sdv /dev/mapper/sdw /dev/mapper/sdx /dev/mapper/sdy /dev/mapper/sdz /dev/mapper/sdaa /dev/mapper/sdab /dev/mapper/sdac /dev/mapper/sdad /dev/mapper/sdae /dev/mapper/sdaf
mdadm: /dev/mapper/sdi appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdj appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdag appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdl appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdm appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdn appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdo appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdp appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdq appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdr appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sds appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdt appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdu appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdv appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdw appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdx appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdy appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdz appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdaa appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdab appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdac appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdad appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdae appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
mdadm: /dev/mapper/sdaf appears to be part of a raid array:
level=container devices=24 ctime=Tue Mar 8 21:18:40 2016
Continue creating array? y
mdadm: Creating array inside ddf container md127
mdadm: array /dev/md1 started.
# cat /proc/mdstat
Personalities : [linear] [multipath] [raid0] [raid1] [raid6] [raid5] [raid4] [raid10]
md1 : active (auto-read-only) raid6 dm-24[23] dm-23[22] dm-22[21] dm-21[20] dm-20[19] dm-19[18] dm-18[17] dm-17[16] dm-16[15] dm-15[14] dm-14[13] dm-13[12] dm-12[11] dm-11[10] dm-10[9] dm-9[8] dm-8[7] dm-7[6] dm-6[5] dm-5[4] dm-4[3] dm-25[2] dm-2[1] dm-1[0]
17208463360 blocks super external:/md127/0 level 6, 64k chunk, algorithm 10 [24/24] [UUUUUUUUUUUUUUUUUUUUUUUU]
md127 : inactive dm-24[23](S) dm-23[22](S) dm-22[21](S) dm-21[20](S) dm-20[19](S) dm-19[18](S) dm-18[17](S) dm-17[16](S) dm-16[15](S) dm-15[14](S) dm-14[13](S) dm-13[12](S) dm-12[11](S) dm-11[10](S) dm-10[9](S) dm-9[8](S) dm-8[7](S) dm-7[6](S) dm-6[5](S) dm-5[4](S) dm-4[3](S) dm-25[2](S) dm-2[1](S) dm-1[0](S)
786432 blocks super external:ddf
md0 : active raid1 sdh1[1] sdg1[0]
124967808 blocks super 1.2 [2/2] [UU]
unused devices: <none>
# mdadm -E /dev/md127
/dev/md127:
Magic : de11de11
Version : 01.02.00
Controller GUID : 4C696E75:782D4D44:2064616D:2D73746F:72616765:2D6E3031
(Linux-MD dam-storage-n01)
Container GUID : 4C696E75:782D4D44:DEADBEEF:00000000:4410E200:C7DA314C
(Linux-MD 03/08/16 21:18:40)
Seq : 00000003
Redundant hdr : yes
Virtual Disks : 1
VD GUID[0] : 4C696E75:782D4D44:DEADBEEF:00000000:4410E257:2EF31542
(Linux-MD 03/08/16 21:20:07)
unit[0] : 1
state[0] : Optimal, Consistent
init state[0] : Fully Initialised
access[0] : Read/Write
Name[0] : 1
Raid Devices[0] : 24 (0@0K 1@0K 2@0K 3@0K 4@0K 5@0K 6@0K 7@0K 8@0K 9@0K 10@0K 11@0K 12@0K 13@0K 14@0K 15@0K 16@0K 17@0K 18@0K 19@0K 20@0K 21@0K 22@0K 23@0K)
Chunk Size[0] : 128 sectors
Raid Level[0] : RAID6
Device Size[0] : 782202880
Array Size[0] : 17208463360
Physical Disks : 1023
Number RefNo Size Device Type/State
0 1bfc5b4f 2930233816K /dev/dm-1 active/Online
1 44f3f4b1 2930233816K /dev/dm-2 active/Online
2 22f1c2e4 2930233816K /dev/dm-25 active/Online
3 ef32ccff 2930233816K /dev/dm-4 active/Online
4 c7e6f1fe 2930233816K /dev/dm-5 active/Online
5 00272b4d 2930233816K /dev/dm-6 active/Online
6 f961cba2 2930233816K /dev/dm-7 active/Online
7 40f01419 2930233816K /dev/dm-8 active/Online
8 75858e24 2930233816K /dev/dm-9 active/Online
9 ac398181 2930233816K /dev/dm-10 active/Online
10 b39d9cbb 2930233816K /dev/dm-11 active/Online
11 a71a4095 2930233816K /dev/dm-12 active/Online
12 f1bf38e9 2930233816K /dev/dm-13 active/Online
13 1a8973b2 2930233816K /dev/dm-14 active/Online
14 c107b1b5 2930233816K /dev/dm-15 active/Online
15 26b44a36 2930233816K /dev/dm-16 active/Online
16 7f376a5f 2930233816K /dev/dm-17 active/Online
17 22944f44 2930233816K /dev/dm-18 active/Online
18 8e356094 2930233816K /dev/dm-19 active/Online
19 0b454914 2930233816K /dev/dm-20 active/Online
20 71df5ccc 2930233816K /dev/dm-21 active/Online
21 763d65a1 2930233816K /dev/dm-22 active/Online
22 aacda00d 2930233816K /dev/dm-23 active/Online
23 9837ac03 2930233816K /dev/dm-24 active/Online
(picking a device at random)
# mdadm -E /dev/mapper/sdp
/dev/mapper/sdp:
Magic : de11de11
Version : 01.02.00
Controller GUID : 4C696E75:782D4D44:2064616D:2D73746F:72616765:2D6E3031
(Linux-MD dam-storage-n01)
Container GUID : 4C696E75:782D4D44:DEADBEEF:00000000:4410E200:C7DA314C
(Linux-MD 03/08/16 21:18:40)
Seq : 00000003
Redundant hdr : yes
Virtual Disks : 1
VD GUID[0] : 4C696E75:782D4D44:DEADBEEF:00000000:4410E257:2EF31542
(Linux-MD 03/08/16 21:20:07)
unit[0] : 1
state[0] : Optimal, Consistent
init state[0] : Fully Initialised
access[0] : Read/Write
Name[0] : 1
Raid Devices[0] : 24 (0@0K 1@0K 2@0K 3@0K 4@0K 5@0K 6@0K 7@0K 8@0K 9@0K 10@0K 11@0K 12@0K 13@0K 14@0K 15@0K 16@0K 17@0K 18@0K 19@0K 20@0K 21@0K 22@0K 23@0K)
Chunk Size[0] : 128 sectors
Raid Level[0] : RAID6
Device Size[0] : 782202880
Array Size[0] : 17208463360
Physical Disks : 1023
Number RefNo Size Device Type/State
0 1bfc5b4f 2930233816K active/Online
1 44f3f4b1 2930233816K active/Online
2 22f1c2e4 2930233816K active/Online
3 ef32ccff 2930233816K active/Online
4 c7e6f1fe 2930233816K active/Online
5 00272b4d 2930233816K active/Online
6 f961cba2 2930233816K active/Online
7 40f01419 2930233816K /dev/dm-8 active/Online
8 75858e24 2930233816K active/Online
9 ac398181 2930233816K active/Online
10 b39d9cbb 2930233816K active/Online
11 a71a4095 2930233816K active/Online
12 f1bf38e9 2930233816K active/Online
13 1a8973b2 2930233816K active/Online
14 c107b1b5 2930233816K active/Online
15 26b44a36 2930233816K active/Online
16 7f376a5f 2930233816K active/Online
17 22944f44 2930233816K active/Online
18 8e356094 2930233816K active/Online
19 0b454914 2930233816K active/Online
20 71df5ccc 2930233816K active/Online
21 763d65a1 2930233816K active/Online
22 aacda00d 2930233816K active/Online
23 9837ac03 2930233816K active/Online
# fdisk -l /dev/md1
Disk /dev/md1: 17621.5 GB, 17621466480640 bytes
2 heads, 4 sectors/track, -1 cylinders, total 34416926720 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 4096 bytes
I/O size (minimum/optimal): 65536 bytes / 1441792 bytes
Disk identifier: 0x00000000
Disk /dev/md1 doesn't contain a valid partition table
# file -s /dev/md1
/dev/md1: LVM2 PV (Linux Logical Volume Manager), UUID: 1lz3cZ-j4Sj-ZTcH-GiEm-eXGi-qMSr-lIzwyS, size: 65999978102784
For reference an array of the same size, attached to the HW RAID controller
# fdisk -l /dev/sda
Disk /dev/sda: 66000.0 GB, 65999989637120 bytes
255 heads, 63 sectors/track, 8024041 cylinders, total 128906229760 sectors
Units = sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 4096 bytes
I/O size (minimum/optimal): 4096 bytes / 4096 bytes
Disk identifier: 0x00000000
Disk /dev/sda doesn't contain a valid partition table
> On Mar 7, 2016, at 4:50 PM, Andreas Klauer <Andreas.Klauer@metamorpher.de> wrote:
>
> On Sun, Mar 06, 2016 at 04:37:42PM -0500, Dan Russell wrote:
>> can md read a RAID6 created by the 2108, or would I be wasting my time?
>
> It might be possible.
>
> I don't use HW-RAID anywhere but if I was forced to, this would be
> one of the first things to determine, how to read it with software
> in case the card fails.
>
>> I can paste or gist the output of madam -E (304 lines) if it would help.
>
> I don't know if it would help, but paste everything you have anyhow.
>
> Is the data encrypted? If not - search for known file type headers
> (like a large jpeg or tar.gz, something larger than disks * chunksize),
> then look at the data on the other disks at the same offsets
> (grab a few megs of each disks), then try to deduce the layout
> and structure from that.
>
> Basically use known data to reverse-engineer the layout if you
> currently know nothing about disk orders, chunk sizes, etc.
>
> Regards
> Andreas Klauer
--
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
^ permalink raw reply
* Re: When do you replace old hard drives in a raid6?
From: John Stoffel @ 2016-03-09 2:49 UTC (permalink / raw)
To: Ram Ramesh; +Cc: Phil Turmel, Linux Raid
In-Reply-To: <56DF6A39.4020904@gmail.com>
>>>>> "Ram" == Ram Ramesh <rramesh2400@gmail.com> writes:
Ram> On 03/06/2016 11:18 PM, Phil Turmel wrote:
>> On 03/06/2016 07:52 PM, Ram Ramesh wrote:
>>> On 03/06/2016 06:29 PM, Phil Turmel wrote:
>>>> On 03/05/2016 03:49 PM, Ram Ramesh wrote:
>>>>> I am curious if people actually replace hard drives periodically because
>>>>> they are old or out of warranty. My 5 device raid6 has several older
>>>>> drives (3/5 are 3+ years old and out of warranty) They seem fine with
>>>>> SMART and raid scrubs. However, it makes me wonder when they will die.
>>>>> What is the best policy in such situations? More importantly, do people
>>>>> wait for disks to die and then replace or have some ad hoc schedule of
>>>>> replacing (like every 6mo replace oldest) to keep things safe?
>>>> I replace drives when their relocation count hits double digits. In my
>>>> limited sample, that's typically after 40,000 hours.
>>>>
>>>> Phil
>>> Thanks for the data point. 40K hours means roughly 4.5 years with 24/7.
>>> That is very good. You use enterprise drives? Mine are desktop (and may
>>> be one HGST NAS)
>> I moved from desktop drives to NAS drives about 4 years ago. So the
>> 40k+ hours were on desktop drives. (A couple started dying in the mid
>> 30,000's, but I suspect I overheated those two.) The oldest NAS drives
>> I have now are approaching 40k, and are all still @ zero relocations.
>> WD Reds, fwiw.
>>
>>> My SMART is perfect except for power on hours. I am going to take it
>>> easy for now as I have a spare (not part of a RAID) just in case
>>> something bad happens.
>> Yes, sounds reasonable.
>>
>> Phil
>>
Ram> My disks have about 10K hours (my server only runs from
Ram> 4pm-2am). I think I have quite a bit of life left assuming an
Ram> on/off cycle is not as bad as extra 14 hours of run time.
The on/off is much worse than just sitting and spinning. That's what
tends to kill drives in my experience. Drives die no matter what, but
laptops and other systems which power off/on tend to die much more
quickly.
John
^ permalink raw reply
* [PATCH] md/raid5: preserve STRIPE_PREREAD_ACTIVE in break_stripe_batch_list
From: NeilBrown @ 2016-03-09 1:58 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, LKML, Martin Svec
[-- Attachment #1: Type: text/plain, Size: 2466 bytes --]
break_stripe_batch_list breaks up a batch and copies some flags from
the batch head to the members, preserving others.
It doesn't preserve or copy STRIPE_PREREAD_ACTIVE. This is not
normally a problem as STRIPE_PREREAD_ACTIVE is cleared when a
stripe_head is added to a batch, and is not set on stripe_heads
already in a batch.
However there is no locking to ensure one thread doesn't set the flag
after it has just been cleared in another. This does occasionally happen.
md/raid5 maintains a count of the number of stripe_heads with
STRIPE_PREREAD_ACTIVE set: conf->preread_active_stripes. When
break_stripe_batch_list clears STRIPE_PREREAD_ACTIVE inadvertently
this could becomes incorrect and will never again return to zero.
md/raid5 delays the handling of some stripe_heads until
preread_active_stripes becomes zero. So when the above mention race
happens, those stripe_heads become blocked and never progress,
resulting is write to the array handing.
So: change break_stripe_batch_list to preserve STRIPE_PREREAD_ACTIVE
in the members of a batch.
URL: https://bugzilla.kernel.org/show_bug.cgi?id=108741
URL: https://bugzilla.redhat.com/show_bug.cgi?id=1258153
URL: http://thread.gmane.org/5649C0E9.2030204@zoner.cz
Reported-by: Martin Svec <martin.svec@zoner.cz> (and others)
Tested-by: Tom Weber <linux@junkyard.4t2.com>
Fixes: 1b956f7a8f9a ("md/raid5: be more selective about distributing flags across batch.")
Cc: stable@vger.kernel.org (v4.1 and later)
Signed-off-by: NeilBrown <neilb@suse.com>
---
drivers/md/raid5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b4f02c9959f2..2e7d253be6ce 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -4236,7 +4236,6 @@ static void break_stripe_batch_list(struct stripe_head *head_sh,
WARN_ON_ONCE(sh->state & ((1 << STRIPE_ACTIVE) |
(1 << STRIPE_SYNCING) |
(1 << STRIPE_REPLACED) |
- (1 << STRIPE_PREREAD_ACTIVE) |
(1 << STRIPE_DELAYED) |
(1 << STRIPE_BIT_DELAY) |
(1 << STRIPE_FULL_WRITE) |
@@ -4251,6 +4250,7 @@ static void break_stripe_batch_list(struct stripe_head *head_sh,
(1 << STRIPE_REPLACED)));
set_mask_bits(&sh->state, ~(STRIPE_EXPAND_SYNC_FLAGS |
+ (1 << STRIPE_PREREAD_ACTIVE) |
(1 << STRIPE_DEGRADED)),
head_sh->state & (1 << STRIPE_INSYNC));
--
2.7.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply related
* Re: RAID6 Array crash during reshape.....now will not re-assemble.
From: NeilBrown @ 2016-03-09 0:23 UTC (permalink / raw)
To: Another Sillyname, Linux-RAID
In-Reply-To: <CAOS+5GEd4E2wzYhw0rPdrZnjK6UwAUdKCQLLTxXrdPO8d7hS7w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7510 bytes --]
On Wed, Mar 02 2016, Another Sillyname wrote:
> I have a 30TB RAID6 array using 7 x 6TB drives that I wanted to
> migrate to RAID5 to take one of the drives offline and use in a new
> array for a migration.
>
> sudo mdadm --grow /dev/md127 --level=raid5 --raid-device=6
> --backup-file=mdadm_backupfile
First observation: Don't use --backup-file unless mdadm tell you that
you have to. New mdadm on new kernel with newly create arrays don't
need a backup file at all. Your array is sufficiently newly created and
I think your mdadm/kernel are new enough too. Note in the --examine output:
> Unused Space : before=262056 sectors, after=143 sectors
This means there is (nearly) 128M of free space in the start of each
device. md can perform the reshape by copying a few chunks down into
this space, then the next few chunks into the space just freed, then the
next few chunks ... and so on. No backup file needed. That is
providing the chunk size is quite a bit smaller than the space, and your
512K chunk size certainly is.
A reshape which increases the size of the array needs 'before' space, a
reshape which decreases the size of the array needs 'after' space. A
reshape which doesn't change the size of the array (like yours) can use
either.
>
> I watched this using cat /proc/mdstat and even after an hour the
> percentage of the reshape was still 0.0%.
A more useful number to watch is the (xxx/yyy) after the percentage.
The first number should change at least every few seconds.
>
> Reboot.....
>
> Array will not come back online at all.
>
> Bring the server up without the array trying to automount.
>
> cat /proc/mdstat shows the array offline.
>
> Personalities :
> md127 : inactive sdf1[2](S) sde1[3](S) sdg1[0](S) sdb1[8](S)
> sdh1[7](S) sdc1[1](S) sdd1[6](S)
> 41022733300 blocks super 1.2
>
> unused devices: <none>
>
> Try to reassemble the array.
>
>>sudo mdadm --assemble /dev/md127 /dev/sdb1 /dev/sdc1 /dev/sdd1 /dev/sde1 /dev/sdf1 /dev/sdg1 /dev/sdh1
> mdadm: /dev/sdg1 is busy - skipping
> mdadm: /dev/sdh1 is busy - skipping
> mdadm: Merging with already-assembled /dev/md/server187.internallan.com:1
It looks like you are getting races with udev. mdadm is detecting the
race and says that it is "Merging" rather than creating a separate array
but still the result isn't very useful...
When you run "mdadm --assemble /dev/md127 ...." mdadm notices that /dev/md127
already exists but isn't active, so it stops it properly so that all the
devices become available to be assembled.
As the devices become available they tell udev "Hey, I've changed
status" and udev says "Hey, you look like part of an md array, let's put
you back together".... or something like that. I might have the details
a little wrong - it is a while since I looked at this.
Anyway it seems that udev called "mdadm -I" to put some of the devices
together so they were busy when your "mdadm --assemble" looked at them.
> mdadm: Failed to restore critical section for reshape, sorry.
> Possibly you needed to specify the --backup-file
>
>
> Have no idea where the server187 stuff has come from.
That is in the 'Name' field in the metadata, which must have been put
there when the array was created
> Name : server187.internallan.com:1
> Creation Time : Sun May 10 14:47:51 2015
It is possible to change it after-the-fact, but unlikely unless someone
explicitly tried.
I doesn't really matter how it got there as all the devices are the
same.
When "mdadm -I /dev/sdb1" etc is run by udev, mdadm needs to deduce a
name for the array. It looks in the Name filed and creates
/dev/md/server187.internallan.com:1
>
> stop the array.
>
>>sudo mdadm --stop /dev/md127
>
> try to re-assemble
>
>>sudo mdadm --assemble /dev/md127 /dev/sdb1 /dev/sdc1 /dev/sdd1 /dev/sde1 /dev/sdf1 /dev/sdg1 /dev/sdh1
>
> mdadm: Failed to restore critical section for reshape, sorry.
> Possibly you needed to specify the --backup-file
>
>
> try to re-assemble using the backup file
>
>>sudo mdadm --assemble /dev/md127 /dev/sdb1 /dev/sdc1 /dev/sdd1 /dev/sde1 /dev/sdf1 /dev/sdg1 /dev/sdh1 --backup-file=mdadm_backupfile
>
> mdadm: Failed to restore critical section for reshape, sorry.
As you have noted else where, the backup file contains nothing useful.
That is causing the problem.
When an in-place reshape like yours (not changing the size of the array,
just changing the configuration) starts the sequence is something like:
- make sure reshape doesn't progress at all (set md/sync_max to zero)
- tell the kernel about the new shape of the array
- start the reshape (this won't make any progress, but will update the
metadata)
Start:
- suspend user-space writes to the next few stripes
- read the next few stripes and write to the backup file
- tell the kernel that it is allowed to progress to the end of those
'few stripes'
- wait for the kernel to do that
- invalidate the backup
- resume user-space writes to those next few stripes
- goto Start
(the process is actually 'double-buffered' so it is more complex, but
this gives the idea close enough)
If the system crashes or is shut down, on restart the kernel cannot know
if the "next few stripes" started reshaping or not, so it depends on
mdadm to load the backup file, check if there is valid data, and write
it out.
I suspect that part of the problem is that mdadm --grow doesn't initialize the
backup file in quite the right way, so when mdadm --assemble looks at it
it doesn't see "Nothing has been written yet" but instead sees
"confusion" and gives up.
If you --stop and then run the same --assemble command, including the
--backup, but this time add --invalid-backup (a bit like Wol
suggested) it should assemble and restart the reshape. --invalid-backup
tells mdadm "I know the backup file is invalid, I know that means there
could be inconsistent data which won't be restored, but I know what is
going on and I'm willing to take that risk. Just don't restore anything,
it'll be find. Really".
I don't actually recommend doing that though.
It would be better to revert the current reshape and start again with no
--backup file. This will use the new mechanism of changing the "Data
Offset" which is easier to work with and should be faster.
If you have the very latest mdadm (3.4) you can add
--update=revert-reshape together with --invalid-backup and in your case
this will cancel the reshape and let you start again.
You can test this out fairly safely if you want to.
mkdir /tmp/foo
mdadm --dump /tmp/foo /dev/.... list of all devices in the array
This will create sparse files in /tmp/foo containing just the md
metadata from those devices. Use "losetup /dev/loop0 /tmp/foo/sdb1" etc
to create loop-back device for all those files (there are multiple hard
links to each file - just choose 1 each).
Then you can experiment with mdadm on those /dev/loopXX files to see
what happens.
Once you have the array reverted, you can start a new --grow, but don't
specify a --backup file. That should DoTheRightThing.
This still leaves the question of why it didn't start a reshape in the
first place. If someone would like to experiment (probably with
loop-back files) and produce a test case that reliably (or even just
occasionally) hangs, then I'm happy to have a look at it.
It also doesn't answer the question of why mdadm doesn't create the
backup file in a format that it knows is safe to ignore. Maybe someone
could look into that.
Good luck :-)
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: When do you replace old hard drives in a raid6?
From: Ram Ramesh @ 2016-03-09 0:11 UTC (permalink / raw)
To: Phil Turmel, Linux Raid
In-Reply-To: <56DD0F12.2060104@turmel.org>
On 03/06/2016 11:18 PM, Phil Turmel wrote:
> On 03/06/2016 07:52 PM, Ram Ramesh wrote:
>> On 03/06/2016 06:29 PM, Phil Turmel wrote:
>>> On 03/05/2016 03:49 PM, Ram Ramesh wrote:
>>>> I am curious if people actually replace hard drives periodically because
>>>> they are old or out of warranty. My 5 device raid6 has several older
>>>> drives (3/5 are 3+ years old and out of warranty) They seem fine with
>>>> SMART and raid scrubs. However, it makes me wonder when they will die.
>>>> What is the best policy in such situations? More importantly, do people
>>>> wait for disks to die and then replace or have some ad hoc schedule of
>>>> replacing (like every 6mo replace oldest) to keep things safe?
>>> I replace drives when their relocation count hits double digits. In my
>>> limited sample, that's typically after 40,000 hours.
>>>
>>> Phil
>> Thanks for the data point. 40K hours means roughly 4.5 years with 24/7.
>> That is very good. You use enterprise drives? Mine are desktop (and may
>> be one HGST NAS)
> I moved from desktop drives to NAS drives about 4 years ago. So the
> 40k+ hours were on desktop drives. (A couple started dying in the mid
> 30,000's, but I suspect I overheated those two.) The oldest NAS drives
> I have now are approaching 40k, and are all still @ zero relocations.
> WD Reds, fwiw.
>
>> My SMART is perfect except for power on hours. I am going to take it
>> easy for now as I have a spare (not part of a RAID) just in case
>> something bad happens.
> Yes, sounds reasonable.
>
> Phil
>
My disks have about 10K hours (my server only runs from 4pm-2am). I
think I have quite a bit of life left
assuming an on/off cycle is not as bad as extra 14 hours of run time.
Ramesh
^ permalink raw reply
* Re: [PATCH 0/8] mdadm static checker fixes
From: NeilBrown @ 2016-03-08 22:55 UTC (permalink / raw)
To: Jes.Sorensen, linux-raid; +Cc: gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]
On Wed, Mar 09 2016, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Hi,
>
> I have been running mdadm through Coverity and fixed a number of
> issues that were raised in the scan. A couple of them were non issues
> related to conditions where we know for sure the kernel will not
> return strings longer than a given size, but there were also a number
> of potential memory leaks and buffer overflows.
>
> These patches are sitting in my pending queue. If you are on the CC
> list, would you mind having a look at the portions touching code you
> previously pushed wrote.
>
> Please hollor if you notice I did anything wrong, otherwise I'll push
> this set into git within the next couple of days.
All
Reviewed-by: NeilBrown <neilb@suse.com>
The comments I have made are only possible enhancements, no problems
found.
I must confess that I was generally fairly careless about resource
leakage. mdadm usually calls 'exit' fairly soon and that releases
everything.
But mdmon and "mdadm --monitor" at long-running so it can pay to be
careful.
And I'm very supportive of silencing warnings from tools that also
provide useful warnings.
Thanks!
NeilBrown
>
> Cheers,
> Jes
>
>
> Jes Sorensen (8):
> Manage: Manage_add(): Fix memory leak
> load_sys(): Add a buffer size argument
> Grow: Grow_continue_command() remove dead code
> Grow: Grow_addbitmap(): Add check to quiet down static code checkers
> {platform,super}-intel: Fix two resource leaks
> bitmap: Fix resource leak in bitmap_file_open()
> Manage: Manage_subdevs() fix file descriptor leak
> super1: Fix potential buffer overflows when copying cluster_name
>
> Detail.c | 2 +-
> Grow.c | 11 ++++++++---
> Manage.c | 8 ++++++--
> bitmap.c | 1 +
> mdadm.h | 2 +-
> platform-intel.c | 7 ++++++-
> super-intel.c | 12 ++++++++----
> super1.c | 18 ++++++++++++------
> sysfs.c | 47 ++++++++++++++++++++++++-----------------------
> 9 files changed, 67 insertions(+), 41 deletions(-)
>
> --
> 2.5.0
>
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH 6/8] bitmap: Fix resource leak in bitmap_file_open()
From: NeilBrown @ 2016-03-08 22:50 UTC (permalink / raw)
To: Jes.Sorensen, linux-raid; +Cc: gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-7-git-send-email-Jes.Sorensen@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
On Wed, Mar 09 2016, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> The code would leak 'fd' if locate_bitmap() failed.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> bitmap.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/bitmap.c b/bitmap.c
> index 5ad7401..0367d13 100644
> --- a/bitmap.c
> +++ b/bitmap.c
> @@ -224,6 +224,7 @@ int bitmap_file_open(char *filename, struct supertype **stp, int node_num)
> } else {
> if (st->ss->locate_bitmap(st, fd, node_num)) {
> pr_err("%s doesn't have bitmap\n", filename);
> + close(fd);
> fd = -1;
> }
> }
Don't you also need a 'close' in
} else if (!st->ss->locate_bitmap) {
pr_err("No bitmap possible with %s metadata\n",
st->ss->name);
return -1;
??
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH 5/8] {platform,super}-intel: Fix two resource leaks
From: NeilBrown @ 2016-03-08 22:45 UTC (permalink / raw)
To: Jes.Sorensen, linux-raid; +Cc: gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-6-git-send-email-Jes.Sorensen@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]
On Wed, Mar 09 2016, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> The code did not free 'dir' allocated by opendir(). An additional
> benefit is that this simplifies the for() loops.
>
> Fixes: 60f0f54d ("IMSM: Add support for VMD")
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> platform-intel.c | 7 ++++++-
> super-intel.c | 6 +++++-
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/platform-intel.c b/platform-intel.c
> index 88818f3..c60fd9e 100644
> --- a/platform-intel.c
> +++ b/platform-intel.c
> @@ -724,8 +724,10 @@ char *vmd_domain_to_controller(struct sys_dev *hba, char *buf)
> return NULL;
>
> dir = opendir("/sys/bus/pci/drivers/vmd");
> + if (!dir)
> + return NULL;
>
> - for (ent = dir ? readdir(dir) : NULL; ent; ent = readdir(dir)) {
> + for (ent = readdir(dir); ent; ent = readdir(dir)) {
> sprintf(path, "/sys/bus/pci/drivers/vmd/%s/domain/device",
> ent->d_name);
>
> @@ -734,8 +736,11 @@ char *vmd_domain_to_controller(struct sys_dev *hba, char *buf)
>
> if (strncmp(buf, hba->path, strlen(buf)) == 0) {
> sprintf(path, "/sys/bus/pci/drivers/vmd/%s", ent->d_name);
> + closedir(dir);
> return realpath(path, buf);
> }
> }
> +
> + closedir(dir);
> return NULL;
> }
> diff --git a/super-intel.c b/super-intel.c
> index 158f4e8..e1bee75 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -1781,7 +1781,10 @@ static int print_vmd_attached_devs(struct sys_dev *hba)
> * this hba
> */
> dir = opendir("/sys/bus/pci/drivers/nvme");
> - for (ent = dir ? readdir(dir) : NULL; ent; ent = readdir(dir)) {
> + if (!dir)
> + return 1;
> +
Returning '1' looks really weird here. I can see it is consistent with
if (hba->type != SYS_DEV_VMD)
return 1;
above, but still....
As the return value is never used, should we just make it 'void' ??
Thanks,
NeilBrown
> + for (ent = readdir(dir); ent; ent = readdir(dir)) {
> int n;
>
> /* is 'ent' a device? check that the 'subsystem' link exists and
> @@ -1814,6 +1817,7 @@ static int print_vmd_attached_devs(struct sys_dev *hba)
> free(rp);
> }
>
> + closedir(dir);
> return 0;
> }
>
> --
> 2.5.0
>
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: [PATCH 3/8] Grow: Grow_continue_command() remove dead code
From: NeilBrown @ 2016-03-08 22:42 UTC (permalink / raw)
To: Jes.Sorensen, linux-raid; +Cc: gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-4-git-send-email-Jes.Sorensen@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]
On Wed, Mar 09 2016, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> All cases where fd2 is used are completed with a close(fd2) ; fd2 = -1;
> so there is no need to check for fd2 > -1 before exiting.
In that case, you can remove all the assignments so -1 to fd2 - if you
like.
Thanks,
NeilBrown
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
> Grow.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index c4f417e..0fa776d 100755
> --- a/Grow.c
> +++ b/Grow.c
> @@ -4924,8 +4924,6 @@ int Grow_continue_command(char *devname, int fd,
> ret_val = Grow_continue(fd, st, content, backup_file, 1, 0);
>
> Grow_continue_command_exit:
> - if (fd2 > -1)
> - close(fd2);
> if (cfd > -1)
> close(cfd);
> st->ss->free_super(st);
> --
> 2.5.0
>
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply
* Re: When do you replace old hard drives in a raid6?
From: Wols Lists @ 2016-03-08 22:01 UTC (permalink / raw)
To: Linux Raid
In-Reply-To: <56DD26DC.8020701@aei.mpg.de>
On 07/03/16 06:59, Carsten Aulbert wrote:
> It really depends a lot on the drive type and manufacturer (for example
> see the various reports by Backblaze).
I've seen it reported that Seagate Barracudas (I remembered it because
that's what I've got) have a bit of a design fault. I think the air
filter has a tendency to leak, and that could be why they fail so quick
once they go - they get dust into them.
Cheers,
Wol
^ permalink raw reply
* [PATCH 8/8] super1: Fix potential buffer overflows when copying cluster_name
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
cmap_get_string() used to retrieve cluster_name does not restrict it's
size. To prevent buffer overflows use the size of the destination
buffer, not strlen() of the source, and null terminate the copied
string.
Fixes: 0aa2f15b ("mdadm: add the ability to change cluster name)"
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
super1.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/super1.c b/super1.c
index f8a35ac..baa9a96 100644
--- a/super1.c
+++ b/super1.c
@@ -2201,6 +2201,7 @@ add_internal_bitmap1(struct supertype *st,
unsigned long long chunk = *chunkp;
int room = 0;
int creating = 0;
+ int len;
struct mdp_superblock_1 *sb = st->sb;
bitmap_super_t *bms = (bitmap_super_t*)(((char*)sb) + MAX_SB_SIZE);
int uuid[4];
@@ -2326,9 +2327,11 @@ add_internal_bitmap1(struct supertype *st,
if (st->nodes)
sb->feature_map = __cpu_to_le32(__le32_to_cpu(sb->feature_map)
| MD_FEATURE_BITMAP_VERSIONED);
- if (st->cluster_name)
- strncpy((char *)bms->cluster_name,
- st->cluster_name, strlen(st->cluster_name));
+ if (st->cluster_name) {
+ len = sizeof(bms->cluster_name);
+ strncpy((char *)bms->cluster_name, st->cluster_name, len);
+ bms->cluster_name[len - 1] = '\0';
+ }
*chunkp = chunk;
return 1;
@@ -2366,7 +2369,7 @@ static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update
bitmap_super_t *bms = (bitmap_super_t*)(((char*)sb)+MAX_SB_SIZE);
int rv = 0;
void *buf;
- int towrite, n;
+ int towrite, n, len;
struct align_fd afd;
unsigned int i = 0;
unsigned long long total_bm_space, bm_space_per_node;
@@ -2375,8 +2378,11 @@ static int write_bitmap1(struct supertype *st, int fd, enum bitmap_update update
case NameUpdate:
/* update cluster name */
if (st->cluster_name) {
- memset((char *)bms->cluster_name, 0, sizeof(bms->cluster_name));
- strncpy((char *)bms->cluster_name, st->cluster_name, 64);
+ len = sizeof(bms->cluster_name);
+ memset((char *)bms->cluster_name, 0, len);
+ strncpy((char *)bms->cluster_name,
+ st->cluster_name, len);
+ bms->cluster_name[len - 1] = '\0';
}
break;
case NodeNumUpdate:
--
2.5.0
^ permalink raw reply related
* [PATCH 7/8] Manage: Manage_subdevs() fix file descriptor leak
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Manage.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Manage.c b/Manage.c
index 9d2e0d0..808c2cb 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1511,9 +1511,10 @@ int Manage_subdevs(char *devname, int fd,
} else {
struct stat stb;
tfd = dev_open(dv->devname, O_RDONLY);
- if (tfd >= 0)
+ if (tfd >= 0) {
fstat(tfd, &stb);
- else {
+ close(tfd);
+ } else {
int open_err = errno;
if (stat(dv->devname, &stb) != 0) {
pr_err("Cannot find %s: %s\n",
--
2.5.0
^ permalink raw reply related
* [PATCH 6/8] bitmap: Fix resource leak in bitmap_file_open()
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
The code would leak 'fd' if locate_bitmap() failed.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
bitmap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/bitmap.c b/bitmap.c
index 5ad7401..0367d13 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -224,6 +224,7 @@ int bitmap_file_open(char *filename, struct supertype **stp, int node_num)
} else {
if (st->ss->locate_bitmap(st, fd, node_num)) {
pr_err("%s doesn't have bitmap\n", filename);
+ close(fd);
fd = -1;
}
}
--
2.5.0
^ permalink raw reply related
* [PATCH 5/8] {platform,super}-intel: Fix two resource leaks
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
The code did not free 'dir' allocated by opendir(). An additional
benefit is that this simplifies the for() loops.
Fixes: 60f0f54d ("IMSM: Add support for VMD")
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
platform-intel.c | 7 ++++++-
super-intel.c | 6 +++++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/platform-intel.c b/platform-intel.c
index 88818f3..c60fd9e 100644
--- a/platform-intel.c
+++ b/platform-intel.c
@@ -724,8 +724,10 @@ char *vmd_domain_to_controller(struct sys_dev *hba, char *buf)
return NULL;
dir = opendir("/sys/bus/pci/drivers/vmd");
+ if (!dir)
+ return NULL;
- for (ent = dir ? readdir(dir) : NULL; ent; ent = readdir(dir)) {
+ for (ent = readdir(dir); ent; ent = readdir(dir)) {
sprintf(path, "/sys/bus/pci/drivers/vmd/%s/domain/device",
ent->d_name);
@@ -734,8 +736,11 @@ char *vmd_domain_to_controller(struct sys_dev *hba, char *buf)
if (strncmp(buf, hba->path, strlen(buf)) == 0) {
sprintf(path, "/sys/bus/pci/drivers/vmd/%s", ent->d_name);
+ closedir(dir);
return realpath(path, buf);
}
}
+
+ closedir(dir);
return NULL;
}
diff --git a/super-intel.c b/super-intel.c
index 158f4e8..e1bee75 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1781,7 +1781,10 @@ static int print_vmd_attached_devs(struct sys_dev *hba)
* this hba
*/
dir = opendir("/sys/bus/pci/drivers/nvme");
- for (ent = dir ? readdir(dir) : NULL; ent; ent = readdir(dir)) {
+ if (!dir)
+ return 1;
+
+ for (ent = readdir(dir); ent; ent = readdir(dir)) {
int n;
/* is 'ent' a device? check that the 'subsystem' link exists and
@@ -1814,6 +1817,7 @@ static int print_vmd_attached_devs(struct sys_dev *hba)
free(rp);
}
+ closedir(dir);
return 0;
}
--
2.5.0
^ permalink raw reply related
* [PATCH 4/8] Grow: Grow_addbitmap(): Add check to quiet down static code checkers
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Grow_addbitmap() is only ever called with s->bitmap_file != NULL, but
not all static code checkers catch this. This adds a check to quiet
down the false positive warnings.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Grow.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Grow.c b/Grow.c
index 0fa776d..c453eb6 100755
--- a/Grow.c
+++ b/Grow.c
@@ -297,7 +297,14 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
" between different architectures. Consider upgrading the Linux kernel.\n");
}
- if (s->bitmap_file && strcmp(s->bitmap_file, "clustered") == 0)
+ /*
+ * We only ever get called if s->bitmap_file is != NULL, so this check
+ * is just here to quiet down static code checkers.
+ */
+ if (!s->bitmap_file)
+ return 1;
+
+ if (strcmp(s->bitmap_file, "clustered") == 0)
major = BITMAP_MAJOR_CLUSTERED;
if (ioctl(fd, GET_BITMAP_FILE, &bmf) != 0) {
--
2.5.0
^ permalink raw reply related
* [PATCH 3/8] Grow: Grow_continue_command() remove dead code
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
All cases where fd2 is used are completed with a close(fd2) ; fd2 = -1;
so there is no need to check for fd2 > -1 before exiting.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Grow.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/Grow.c b/Grow.c
index c4f417e..0fa776d 100755
--- a/Grow.c
+++ b/Grow.c
@@ -4924,8 +4924,6 @@ int Grow_continue_command(char *devname, int fd,
ret_val = Grow_continue(fd, st, content, backup_file, 1, 0);
Grow_continue_command_exit:
- if (fd2 > -1)
- close(fd2);
if (cfd > -1)
close(cfd);
st->ss->free_super(st);
--
2.5.0
^ permalink raw reply related
* [PATCH 2/8] load_sys(): Add a buffer size argument
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
This adds a buffer size argument to load_sys(), rather than relying on
a hard coded buffer size. The old behavior was safe because we knew
the kernel would never return strings overrunning the buffers, however
it was ugly, and would cause code checking tools to spit out warnings.
This caused a Coverity warning over the read into
sra->sysfs_array_state which is only 20 bytes.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Detail.c | 2 +-
mdadm.h | 2 +-
super-intel.c | 6 +++---
sysfs.c | 47 ++++++++++++++++++++++++-----------------------
4 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/Detail.c b/Detail.c
index 0cfccad..20c4553 100644
--- a/Detail.c
+++ b/Detail.c
@@ -582,7 +582,7 @@ This is pretty boring
continue;
sprintf(path, "/sys/block/%s/md/metadata_version",
de->d_name);
- if (load_sys(path, vbuf) < 0)
+ if (load_sys(path, vbuf, sizeof(vbuf)) < 0)
continue;
if (strncmp(vbuf, "external:", 9) != 0 ||
!is_subarray(vbuf+9) ||
diff --git a/mdadm.h b/mdadm.h
index 97aac52..3b96076 100755
--- a/mdadm.h
+++ b/mdadm.h
@@ -631,7 +631,7 @@ extern int sysfs_disk_to_scsi_id(int fd, __u32 *id);
extern int sysfs_unique_holder(char *devnm, long rdev);
extern int sysfs_freeze_array(struct mdinfo *sra);
extern int sysfs_wait(int fd, int *msec);
-extern int load_sys(char *path, char *buf);
+extern int load_sys(char *path, char *buf, int len);
extern int reshape_prepare_fdlist(char *devname,
struct mdinfo *sra,
int raid_disks,
diff --git a/super-intel.c b/super-intel.c
index ff0506d..158f4e8 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1654,7 +1654,7 @@ static int ahci_enumerate_ports(const char *hba_path, int port_count, int host_b
break;
}
sprintf(device, "/sys/dev/block/%d:%d/device/type", major, minor);
- if (load_sys(device, buf) != 0) {
+ if (load_sys(device, buf, sizeof(buf)) != 0) {
if (verbose > 0)
pr_err("failed to read device type for %s\n",
path);
@@ -1669,7 +1669,7 @@ static int ahci_enumerate_ports(const char *hba_path, int port_count, int host_b
vendor[0] = '\0';
model[0] = '\0';
sprintf(device, "/sys/dev/block/%d:%d/device/vendor", major, minor);
- if (load_sys(device, buf) == 0) {
+ if (load_sys(device, buf, sizeof(buf)) == 0) {
strncpy(vendor, buf, sizeof(vendor));
vendor[sizeof(vendor) - 1] = '\0';
c = (char *) &vendor[sizeof(vendor) - 1];
@@ -1678,7 +1678,7 @@ static int ahci_enumerate_ports(const char *hba_path, int port_count, int host_b
}
sprintf(device, "/sys/dev/block/%d:%d/device/model", major, minor);
- if (load_sys(device, buf) == 0) {
+ if (load_sys(device, buf, sizeof(buf)) == 0) {
strncpy(model, buf, sizeof(model));
model[sizeof(model) - 1] = '\0';
c = (char *) &model[sizeof(model) - 1];
diff --git a/sysfs.c b/sysfs.c
index 2600343..8379ca8 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -27,15 +27,15 @@
#include <dirent.h>
#include <ctype.h>
-int load_sys(char *path, char *buf)
+int load_sys(char *path, char *buf, int len)
{
int fd = open(path, O_RDONLY);
int n;
if (fd < 0)
return -1;
- n = read(fd, buf, 1024);
+ n = read(fd, buf, len);
close(fd);
- if (n <0 || n >= 1024)
+ if (n <0 || n >= len)
return -1;
buf[n] = 0;
if (n && buf[n-1] == '\n')
@@ -118,7 +118,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
sra->devs = NULL;
if (options & GET_VERSION) {
strcpy(base, "metadata_version");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
if (strncmp(buf, "none", 4) == 0) {
sra->array.major_version =
@@ -137,31 +137,31 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
}
if (options & GET_LEVEL) {
strcpy(base, "level");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
sra->array.level = map_name(pers, buf);
}
if (options & GET_LAYOUT) {
strcpy(base, "layout");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
sra->array.layout = strtoul(buf, NULL, 0);
}
if (options & GET_DISKS) {
strcpy(base, "raid_disks");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
sra->array.raid_disks = strtoul(buf, NULL, 0);
}
if (options & GET_DEGRADED) {
strcpy(base, "degraded");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
sra->array.failed_disks = strtoul(buf, NULL, 0);
}
if (options & GET_COMPONENT) {
strcpy(base, "component_size");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
sra->component_size = strtoull(buf, NULL, 0);
/* sysfs reports "K", but we want sectors */
@@ -169,13 +169,13 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
}
if (options & GET_CHUNK) {
strcpy(base, "chunk_size");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
sra->array.chunk_size = strtoul(buf, NULL, 0);
}
if (options & GET_CACHE) {
strcpy(base, "stripe_cache_size");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
/* Probably level doesn't support it */
sra->cache_size = 0;
else
@@ -183,7 +183,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
}
if (options & GET_MISMATCH) {
strcpy(base, "mismatch_cnt");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
sra->mismatch_cnt = strtoul(buf, NULL, 0);
}
@@ -195,7 +195,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
size_t len;
strcpy(base, "safe_mode_delay");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
/* remove a period, and count digits after it */
@@ -218,7 +218,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
}
if (options & GET_BITMAP_LOCATION) {
strcpy(base, "bitmap/location");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
if (strncmp(buf, "file", 4) == 0)
sra->bitmap_offset = 1;
@@ -232,7 +232,8 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
if (options & GET_ARRAY_STATE) {
strcpy(base, "array_state");
- if (load_sys(fname, sra->sysfs_array_state))
+ if (load_sys(fname, sra->sysfs_array_state,
+ sizeof(sra->sysfs_array_state)))
goto abort;
} else
sra->sysfs_array_state[0] = 0;
@@ -262,7 +263,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
/* Always get slot, major, minor */
strcpy(dbase, "slot");
- if (load_sys(fname, buf)) {
+ if (load_sys(fname, buf, sizeof(buf))) {
/* hmm... unable to read 'slot' maybe the device
* is going away?
*/
@@ -287,7 +288,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
if (*ep) dev->disk.raid_disk = -1;
strcpy(dbase, "block/dev");
- if (load_sys(fname, buf)) {
+ if (load_sys(fname, buf, sizeof(buf))) {
/* assume this is a stale reference to a hot
* removed device
*/
@@ -299,7 +300,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
/* special case check for block devices that can go 'offline' */
strcpy(dbase, "block/device/state");
- if (load_sys(fname, buf) == 0 &&
+ if (load_sys(fname, buf, sizeof(buf)) == 0 &&
strncmp(buf, "offline", 7) == 0) {
free(dev);
continue;
@@ -312,25 +313,25 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
if (options & GET_OFFSET) {
strcpy(dbase, "offset");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
dev->data_offset = strtoull(buf, NULL, 0);
strcpy(dbase, "new_offset");
- if (load_sys(fname, buf) == 0)
+ if (load_sys(fname, buf, sizeof(buf)) == 0)
dev->new_data_offset = strtoull(buf, NULL, 0);
else
dev->new_data_offset = dev->data_offset;
}
if (options & GET_SIZE) {
strcpy(dbase, "size");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
dev->component_size = strtoull(buf, NULL, 0) * 2;
}
if (options & GET_STATE) {
dev->disk.state = 0;
strcpy(dbase, "state");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
if (strstr(buf, "in_sync"))
dev->disk.state |= (1<<MD_DISK_SYNC);
@@ -341,7 +342,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
}
if (options & GET_ERROR) {
strcpy(buf, "errors");
- if (load_sys(fname, buf))
+ if (load_sys(fname, buf, sizeof(buf)))
goto abort;
dev->errors = strtoul(buf, NULL, 0);
}
--
2.5.0
^ permalink raw reply related
* [PATCH 1/8] Manage: Manage_add(): Fix memory leak
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
In-Reply-To: <1457458252-20203-1-git-send-email-Jes.Sorensen@redhat.com>
From: Jes Sorensen <Jes.Sorensen@redhat.com>
sysfs_read() allocates and populates a struct mdinfo, however the code
forgot to free it again, before dropping the reference to the pointer.
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Manage.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Manage.c b/Manage.c
index a812ba0..9d2e0d0 100644
--- a/Manage.c
+++ b/Manage.c
@@ -944,10 +944,13 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
}
if (strncmp(mdp->sysfs_array_state, "readonly", 8) != 0) {
+ sysfs_free(mdp);
pr_err("%s is not readonly, cannot add journal.\n", devname);
return -1;
}
+ sysfs_free(mdp);
+
tst->ss->getinfo_super(tst, &mdi, NULL);
if (mdi.journal_device_required == 0) {
pr_err("%s does not support journal device.\n", devname);
--
2.5.0
^ permalink raw reply related
* [PATCH 0/8] mdadm static checker fixes
From: Jes.Sorensen @ 2016-03-08 17:30 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, gqjiang, pawel.baldysiak
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Hi,
I have been running mdadm through Coverity and fixed a number of
issues that were raised in the scan. A couple of them were non issues
related to conditions where we know for sure the kernel will not
return strings longer than a given size, but there were also a number
of potential memory leaks and buffer overflows.
These patches are sitting in my pending queue. If you are on the CC
list, would you mind having a look at the portions touching code you
previously pushed wrote.
Please hollor if you notice I did anything wrong, otherwise I'll push
this set into git within the next couple of days.
Cheers,
Jes
Jes Sorensen (8):
Manage: Manage_add(): Fix memory leak
load_sys(): Add a buffer size argument
Grow: Grow_continue_command() remove dead code
Grow: Grow_addbitmap(): Add check to quiet down static code checkers
{platform,super}-intel: Fix two resource leaks
bitmap: Fix resource leak in bitmap_file_open()
Manage: Manage_subdevs() fix file descriptor leak
super1: Fix potential buffer overflows when copying cluster_name
Detail.c | 2 +-
Grow.c | 11 ++++++++---
Manage.c | 8 ++++++--
bitmap.c | 1 +
mdadm.h | 2 +-
platform-intel.c | 7 ++++++-
super-intel.c | 12 ++++++++----
super1.c | 18 ++++++++++++------
sysfs.c | 47 ++++++++++++++++++++++++-----------------------
9 files changed, 67 insertions(+), 41 deletions(-)
--
2.5.0
^ 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