linux-kernel - Re: [PATCH 6/6] ipc/sem.c: make semctl(,,{GETNCNT,GETZCNT}) standard compliant

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: <1400079158.9356.10.camel@buesod1.americas.hpqcorp.net>
Date:	Wed, 14 May 2014 07:52:38 -0700
From:	Davidlohr Bueso <davidlohr@...com>
To:	Manfred Spraul <manfred@...orfullife.com>
Cc:	Davidlohr Bueso <davidlohr.bueso@...com>,
	Michael Kerrisk <mtk.manpages@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>, 1vier1@....de
Subject: Re: [PATCH 6/6] ipc/sem.c: make semctl(,,{GETNCNT,GETZCNT})
 standard compliant

On Sat, 2014-05-10 at 12:03 +0200, Manfred Spraul wrote:
> Per definition, a task waits on exactly one semaphore:
> The semaphore from the first operation in the sop array that cannot proceed.
> 
> The Linux implementation never followed the standard, it tried to count all
> semaphores that might be the reason why a task sleeps.
> 
> This patch fixes that.
> 
> Note:
> The implementation assumes that GETNCNT and GETZCNT are rare operations,
> therefore the code counts them only on demand.
> (If they wouldn't be rare, then the non-compliance would have
> been found earlier)
> 
> Signed-off-by: Manfred Spraul <manfred@...orfullife.com>
> ---
>  ipc/sem.c | 37 ++++++++++++++++---------------------
>  1 file changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 22a4c12..5e8bcde 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -993,38 +993,33 @@ static void do_smart_update(struct sem_array *sma, struct sembuf *sops, int nsop
>  }
>  
>  /*
> - * check_qop: Test how often a queued operation sleeps on the semaphore semnum
> + * check_qop: Test if a queued operation sleeps on the semaphore semnum
>   */
>  static int check_qop(struct sem_array *sma, int semnum, struct sem_queue *q,
>  			bool count_zero)
>  {
> -	struct sembuf *sops = q->sops;
> -	int nsops = q->nsops;
> -	int i, semcnt;
> +	struct sembuf *sop = q->blocking;
>  
> -	semcnt = 0;
> +	BUG_ON(sop->sem_flg & IPC_NOWAIT);
> +	BUG_ON(sop->sem_op > 0);

Hmm in light of Linus' recent criticism about randomly sprinkling
BUG_ONs in the kernel I'm not sure we want this. Yes, all those calls
are correct from a logical pov and should never occur, however, would
WARN be more suitable instead? I don't know. 

Andrew, any thoughts?

Thanks.

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