Messages in this thread | | | From | "xiaoguangrong(Xiao Guangrong)" <> | Subject | Re: [PATCH] kfifo: add memory barrier in kfifo to prevent data loss | Date | Thu, 3 Jan 2019 07:43:10 +0000 |
| |
On 12/12/18 8:50 AM, Kees Cook wrote: > On Mon, Dec 10, 2018 at 7:41 PM <yulei.kernel@gmail.com> wrote: >> >> From: Yulei Zhang <yuleixzhang@tencent.com> >> >> Early this year we spot there may be two issues in kernel >> kfifo. >> >> One is reported by Xiao Guangrong to linux kernel. >> https://lkml.org/lkml/2018/5/11/58 >> In current kfifo implementation there are missing memory >> barrier in the read side, so that without proper barrier >> between reading the kfifo->in and fetching the data there >> is potential ordering issue. >> >> Beside that, there is another potential issue in kfifo, >> please consider the following case: >> at the beginning >> ring->size = 4 >> ring->out = 0 >> ring->in = 4 >> >> Consumer Producer >> --------------- -------------- >> index = ring->out; /* index == 0 */ >> ring->out++; /* ring->out == 1 */ >> < Re-Order > >> out = ring->out; >> if (ring->in - out >= ring->mask) >> return -EFULL; >> /* see the ring is not full */ >> index = ring->in & ring->mask; >> /* index == 0 */ >> ring->data[index] = new_data; >> ring->in++; >> >> data = ring->data[index]; >> /* you will find the old data is overwritten by the new_data */ >> >> In order to avoid the issue: >> 1) for the consumer, we should read the ring->data[] out before >> updating ring->out >> 2) for the producer, we should read ring->out before updating >> ring->data[] >> >> So in this patch we introduce the following four functions which >> are wrapped with proper memory barrier and keep in pairs to make >> sure the in and out index are fetched and updated in order to avoid >> data loss. >> >> kfifo_read_index_in() >> kfifo_write_index_in() >> kfifo_read_index_out() >> kfifo_write_index_out() >> >> Signed-off-by: Yulei Zhang <yuleixzhang@tencent.com> >> Signed-off-by: Guangrong Xiao <xiaoguangrong@tencent.com> > > I've added some more people to CC that might want to see this. Thanks > for sending this!
Hi,
Ping... could anyone have a look? ;)
Thanks!
| |