RE: [PATCH] tty: hold lock across tty buffer finding and bufferfilling

[Du, Alek]

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@xxxxxxxxx] 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@xxxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/