linux-kernel - RE: [PATCH] tty: hold lock across tty buffer finding and buffer filling

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:	Fri, 16 Mar 2012 10:01:17 +0000
From:	"Du, Alek" <alek.du@...el.com>
To:	Jiri Slaby <jslaby@...e.cz>
CC:	Jiri Slaby <jirislaby@...il.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	"Tu, Xiaobing" <xiaobing.tu@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"Zhang, Yanmin" <yanmin.zhang@...el.com>,
	"Zuo, Jiao" <jiao.zuo@...el.com>
Subject: RE: [PATCH] tty: hold lock across tty buffer finding and buffer
 filling

We prepared the buffer, and use it. And during this, we don't release the spinlock of buf.lock, how the flush could happen?

               A
spin_lock
prepare the buffer
user the buffer
spin unlock


              B
Spin_lock
Flush
Free the buffer
Put buf.tail = NULL
Spin unlock



Thanks,
Alek
-----Original Message-----
From: Jiri Slaby [mailto:jirislaby@...il.com] On Behalf Of Jiri Slaby
Sent: Friday, March 16, 2012 5:57 PM
To: Du, Alek
Cc: Jiri Slaby; Alan Cox; Tu, Xiaobing; linux-kernel@...r.kernel.org; gregkh@...uxfoundation.org; Zhang, Yanmin; Zuo, Jiao
Subject: Re: [PATCH] tty: hold lock across tty buffer finding and buffer filling

On 03/16/2012 10:49 AM, Du, Alek wrote:
> The tty structure is protect and not null, the tty->buf.tail is null. 
> Many people think the tty reference count isn't protected and cause 
> this bug, it is not true. :-)

You protect the pointer, not what is inside that structure. If you increment a tty reference count, tty->buf.tail won't be NULL iff tty is not NULL. IOW, you have to use tty_port_tty_set/get all around.

> For the flush case, it need acquire the spinlock to free the buffer and put buf.tail to NULL. So this patch will help:
> 
> Here is the example place, you can see the __tty_buffer_flush is inside the spinlock.

I'm not talking about flush. I'm talking about prepare. It returns a pointer to a tty buffer which may be concurrently freed by flush.

     A                                   B
================================================
prepare(&chars);
                                        flush(); memcpy(chars, data_from_HW); <- chars is freed now flip();

thanks,
--
js
suse labs
--
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/