linux-kernel - Re: [patch 4/7] vfs: mountinfo: add mount peer group ID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080326223949.GU10722@ZenIV.linux.org.uk>
Date:	Wed, 26 Mar 2008 22:39:50 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	akpm@...ux-foundation.org, linuxram@...ibm.com,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [patch 4/7] vfs: mountinfo: add mount peer group ID

On Wed, Mar 26, 2008 at 10:11:35PM +0100, Miklos Szeredi wrote:
> +static int mnt_alloc_group_id(struct vfsmount *mnt)
> +{
> +	int res;
> +
> +	WARN_ON(mnt->mnt_group_id != 0);
> + retry:
> +	res = ida_get_new_above(&mnt_group_ida, 1, &mnt->mnt_group_id);
> +	if (res == -EAGAIN) {
> +		if (ida_pre_get(&mnt_group_ida, GFP_KERNEL))
> +			goto retry;
> +		res = -ENOMEM;
> +	}
> +	return res;
> +}
> +

Same comment on goto retry

> +/*
> + * Release a peer group ID
> + */
> +void mnt_release_group_id(struct vfsmount *mnt)
> +{
> +	ida_remove(&mnt_group_ida, mnt->mnt_group_id);
> +	mnt->mnt_group_id = 0;
> +}

Erm...  What if that sucker has zero group ID to start with?  You
do call it that way later.

> +static void cleanup_group_ids(struct vfsmount *mnt, struct vfsmount *end)
> +{
> +	struct vfsmount *p;
> +
> +	for (p = mnt; p != end; p = next_mnt(p, mnt)) {
> +		if (list_empty(&p->mnt_share) && !IS_MNT_SHARED(p))
> +			mnt_release_group_id(p);
> +	}
> +}

I'd make that p->mnt_group_id && !IS_MNT_SHARED(p), actually...

> +static int invent_group_ids(struct vfsmount *mnt, bool recurse)
> +{
> +	struct vfsmount *p;
> +
> +	for (p = mnt; p; p = recurse ? next_mnt(p, mnt) : NULL) {
> +		if (list_empty(&p->mnt_share) && !IS_MNT_SHARED(p)) {
> +			int err = mnt_alloc_group_id(p);
> +			if (err) {
> +				cleanup_group_ids(mnt, p);
> +				return err;
> +			}
> +		}
> +	}

Same comment

> @@ -58,9 +62,9 @@ static int do_make_slave(struct vfsmount
>  		list_splice(&mnt->mnt_slave_list, master->mnt_slave_list.prev);
>  		INIT_LIST_HEAD(&mnt->mnt_slave_list);
>  	} else {
> -		struct list_head *p = &mnt->mnt_slave_list;
> -		while (!list_empty(p)) {
> -                        slave_mnt = list_first_entry(p,
> +		struct list_head *slaves = &mnt->mnt_slave_list;
> +		while (!list_empty(slaves)) {
> +                        slave_mnt = list_first_entry(slaves,
>  					struct vfsmount, mnt_slave);
>  			list_del_init(&slave_mnt->mnt_slave);
>  			slave_mnt->mnt_master = NULL;

What is that renaming doing here?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/