Re: [PATCH 3.14 17/29] sysv, ipc: fix security-layer leaking

[Willy Tarreau <w@1wt.eu>]

Hi Manfred,

On Mon, Aug 29, 2016 at 11:23:55AM +0200, Manfred Spraul wrote:
> Hi Willy,
> 
> On 08/21/2016 01:49 PM, Willy Tarreau wrote:
> > Hi guys,
> > 
> > On Sun, Aug 14, 2016 at 10:07:45PM +0200, Greg Kroah-Hartman wrote:
> > > 3.14-stable review patch.  If anyone has any objections, please let me know.
> > > 
> > > ------------------
> > > 
> > > From: Fabian Frederick <fabf@skynet.be>
> > > 
> > > commit 9b24fef9f0410fb5364245d6cc2bd044cc064007 upstream.
> > > 
> > > Commit 53dad6d3a8e5 ("ipc: fix race with LSMs") updated ipc_rcu_putref()
> > > to receive rcu freeing function but used generic ipc_rcu_free() instead
> > > of msg_rcu_free() which does security cleaning.
> > > 
> > > Running LTP msgsnd06 with kmemleak gives the following:
> > > 
> > >    cat /sys/kernel/debug/kmemleak
> > > 
> > >    unreferenced object 0xffff88003c0a11f8 (size 8):
> > >      comm "msgsnd06", pid 1645, jiffies 4294672526 (age 6.549s)
> > >      hex dump (first 8 bytes):
> > >        1b 00 00 00 01 00 00 00                          ........
> > >      backtrace:
> > >        kmemleak_alloc+0x23/0x40
> > >        kmem_cache_alloc_trace+0xe1/0x180
> > >        selinux_msg_queue_alloc_security+0x3f/0xd0
> > >        security_msg_queue_alloc+0x2e/0x40
> > >        newque+0x4e/0x150
> > >        ipcget+0x159/0x1b0
> > >        SyS_msgget+0x39/0x40
> > >        entry_SYSCALL_64_fastpath+0x13/0x8f
> > > 
> > > Manfred Spraul suggested to fix sem.c as well and Davidlohr Bueso to
> > > only use ipc_rcu_free in case of security allocation failure in newary()
> > > 
> > > Fixes: 53dad6d3a8e ("ipc: fix race with LSMs")
> > > Link: http://lkml.kernel.org/r/1470083552-22966-1-git-send-email-fabf@skynet.be
> > > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > > Cc: Davidlohr Bueso <dbueso@suse.de>
> > > Cc: Manfred Spraul <manfred@colorfullife.com>
> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > The patch above was tagged for stable v3.12+, however it references a fix
> > that was backported in 3.10.16 as commit e84ca333, so I'm unsure whether
> > 3.10 is affected or not. It *seems* to me that I should replace remaining
> > instances of ipc_rcu_free with sem_rcu_free in sem.c, and with msg_rcu_free
> > in msg.c, but I'd prefer a confirmation. For now I'm postponing this fix,
> > any hint would be much appreciated.
> Yes, we need the patch for v3.10 as well.
> There must be exactly two instances of ipc_rcu_free in each of sem.c, msg.c,
> shm.c:
> It is called when security_{sem,msg_queue,shm}_alloc fails.
> And obviously within sem_rcu_free, ... .
> 
> The rest must be sem_rcu_free(), ... .

OK that seems clear enough.

> Should I test if the patch from 3.14 works with v3.10?

As you like. Your explanation seems clear to me, I think I'll get it
right. However if you have an easy reproducer and don't mind testing
it on 3.10.103, that would indeed be appreciated, otherwise do not
waste your time.

thanks,
Willy