linux-kernel - Re: [PATCH] ipc/sem.c: prevent ENOMEM in semop() w/ SEM

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CALgW_8V=E0kuArcFCUXTOuNiay794Nd8tge=T65q0Fxp2Wnaow@mail.gmail.com>
Date:	Tue, 7 Aug 2012 13:10:07 +0200
From:	Manfred Spraul <manfred@...orfullife.com>
To:	Seiichi Ikarashi <s.ikarashi@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ipc/sem.c: prevent ENOMEM in semop() w/ SEM_UNDO flag

Hi Seiichi,

2012/8/6 Seiichi Ikarashi <s.ikarashi@...fujitsu.com>
>
>
>  A real case was as follows.
>      semget(IPC_PRIVATE, 70000, IPC_CREAT | IPC_EXCL);
>      sops[0].sem_num = 0;
>      sops[0].sem_op  = 1;
>      sops[0].sem_flg = SEM_UNDO;
>      semop(semid, sops, 1);
>

I think this can't work: sops[].sem_num is defined as "unsigned short".
Thus more than 65500 semaphores in one semaphore set do not make
any sense.
"unsigned short" is also specified in the opengroup standard:

http://pubs.opengroup.org/onlinepubs/7908799/xsh/syssem.h.html

Thus: The hard limit is 65535. Perhaps slightly less, I haven't checked
if (-1) is used somewhere to indicate an error.

Is it possible to split the semaphores into multiple semphore ids?
e.g. 70 ids, each with 1000 semaphores?

The atomicity would be lost (e.g. all SEM_UNDO operations within
one id are performed at once. With 70 ids, the SEM_UNDOs are not
atomic anymore)

>
>    #define SEMMSL  250             /* <= 8 000 max num of semaphores per id */
>

As far as I can see your patch removes the last part of the code that
caused the restriction to 8.000 semaphores per id.

Thus I'd propose that your patch changes this line to

+ #define SEMMSL  250             /* <= 65 500 max num of semaphores per id */

And:
I would add a comment into the patch description all semaphores
from one id share a single kernel spinlock. This could be changed, but
it would
a) add complexity for all users and
b) change user space visible behavior
Thus I would prefer to avoid to implement it unless there are real
applications that need this implementation.

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