linux-kernel - Re: [PATCH v2] ipc: introduce ipc_valid_object() helper to sort out IPC

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]

Date:	Wed, 18 Dec 2013 20:00:36 +0100
From:	Manfred Spraul <manfred@...orfullife.com>
To:	Rafael Aquini <aquini@...hat.com>,
	Davidlohr Bueso <davidlohr@...com>
CC:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>,
	Greg Thelen <gthelen@...gle.com>
Subject: Re: [PATCH v2] ipc: introduce ipc_valid_object() helper to sort out
 IPC_RMID races

Hi Rafael,

On 12/18/2013 06:34 PM, Rafael Aquini wrote:
> Folks,
>
> Before I re-submit the v3 with the commentary changes requested, I'm pasting
> here what I'm planning to amend to v2 patch:
> ---
> diff --git a/ipc/sem.c b/ipc/sem.c
> index ed0057a..23379b6 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -1846,6 +1846,14 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __u
>   
>          error = -EIDRM;
>          locknum = sem_lock(sma, sops, nsops);
> +       /*
> +        * We eventually might perform the following check in a lockless
> +        * fashion here, considering ipc_valid_object() locking constraints.
> +        * If nsops == 1 and there's no contention for sem_perm.lock, then
> +        * only a per-semaphore lock is held and it's OK to go on the check
> +        * below. More details on the fine grained locking scheme entangled
> +        * here, and why it's RMID race safe on comments at sem_lock()
> +        */
>          if (!ipc_valid_object(&sma->sem_perm))
>                  goto out_unlock_free;
>          /*
> diff --git a/ipc/util.h b/ipc/util.h
> index 071ed58..d05b708 100644
> --- a/ipc/util.h
> +++ b/ipc/util.h
> @@ -190,7 +190,8 @@ static inline void ipc_unlock(struct kern_ipc_perm *perm)
>    * where the respective ipc_ids.rwsem is not being held down.
>    * Checks whether the ipc object is still around or if it's gone already, as
>    * ipc_rmid() may have already freed the ID while the ipc lock was spinning.
> - * Needs to be called with kern_ipc_perm.lock held.
> + * Needs to be called with kern_ipc_perm.lock held -- exception made for one
> + * checkpoint case at sys_semtimedop() as noted in code commentary.
>    */
>   static inline bool ipc_valid_object(struct kern_ipc_perm *perm)
>   {
> ---
>
> Do we need to change somthing else?
> Looking forward your thoughts!
Acked-by: Manfred Spraul <manfred@...orfullife.com>

--
     Manfred
--
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/