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:   Sat, 2 Dec 2017 07:03:30 +0100
From:   Manfred Spraul <>
To:     Davidlohr Bueso <>,
        Philippe Mikoyan <>
Subject: Re: [PATCH 2/2] ipc: Fix ipc data structures inconsistency


On 12/01/2017 06:20 PM, Davidlohr Bueso wrote:
> On Thu, 30 Nov 2017, Philippe Mikoyan wrote:
>> As described in the title, this patch fixes <ipc>id_ds inconsistency
>> when <ipc>ctl_stat runs concurrently with some ds-changing function,
>> e.g. shmat, msgsnd or whatever.
>> For instance, if shmctl(IPC_STAT) is running concurrently with shmat,
>> following data structure can be returned:
>> {... shm_lpid = 0, shm_nattch = 1, ...}
The patch appears to be good. I'll try to perform some tests, but I'm 
not sure when I will be able to.
Especially: I don't know the shm code good enough to immediately check 
the change you make to nattach.

And, perhaps as a side information:
There appears to be a use-after-free in shm, I now got a 2nd mail from 

> Hmm yeah that's pretty fishy, also shm_atime = 0, no?
> So I think this patch is fine as we can obviously race at a user level.
> This is another justification for converting the ipc lock to rwlock;
> performance wise they are the pretty much the same (being queued)...
> but that's irrelevant to this patch. I like that you manage to do
> security and such checks still only under rcu, like all ipc calls
> work; *_stat() is no longer special.
I don't like rwlock, they add complexity without reducing the cache line 

What I would like to try is to create a mutex_lock_rcu() function, and 
then convert everything to a mutex.

As pseudocode::
     if (failed) {
     } else {

Obviously, the getref then within the mutex framework, i.e. only if 
mutex_lock() really sleeps.
If the code in ipc gets significantly simpler, then perhaps convert it 
to an rw mutex.

Powered by blists - more mailing lists