watch_queue: Fix lack of barrier/sync/lock between post and read

There's nothing to synchronise post_one_notification() versus pipe_read(). Whilst posting is done under pipe->rd_wait.lock, the reader only takes pipe->mutex which cannot bar notification posting as that may need to be made from contexts that cannot sleep. Fix this by setting pipe->head with a barrier in post_one_notification() and reading pipe->head with a barrier in pipe_read(). If that's not sufficient, the rd_wait.lock will need to be taken, possibly in a ->confirm() op so that it only applies to notifications. The lock would, however, have to be dropped before copy_page_to_iter() is invoked.

David Howells committed 4 years ago

Commit 2ed147f1 parent 7ea1a012 files · +3 −2Browse files

Unified · Split

2 files

fs/pipe.c +2 −1
kernel/watch_queue.c +1 −1

fs/pipe.c+2−1

@@ -253,7 +253,8 @@ pipe_read(struct kiocb iocb, struct iov_iter to) · 252 unmodified lines
253	*/	253	*/
254	was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);	254	was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
255	for (;;) {	255	for (;;) {
256	unsigned int head = pipe->head;	256	/* Read ->head with a barrier vs post_one_notification() */
		257	unsigned int head = smp_load_acquire(&pipe->head);
257	unsigned int tail = pipe->tail;	258	unsigned int tail = pipe->tail;
258	unsigned int mask = pipe->ring_size - 1;	259	unsigned int mask = pipe->ring_size - 1;
259		260

kernel/watch_queue.c+1−1

@@ -113,7 +113,7 @@ static bool post_one_notification(struct watch_queue *wqueue, · 112 unmodified lines
113	buf->offset = offset;	113	buf->offset = offset;
114	buf->len = len;	114	buf->len = len;
115	buf->flags = PIPE_BUF_FLAG_WHOLE;	115	buf->flags = PIPE_BUF_FLAG_WHOLE;
116	pipe->head = head + 1;	116	smp_store_release(&pipe->head, head + 1); /* vs pipe_read() */
117		117
118	if (!test_and_clear_bit(note, wqueue->notes_bitmap)) {	118	if (!test_and_clear_bit(note, wqueue->notes_bitmap)) {
119	spin_unlock_irq(&pipe->rd_wait.lock);	119	spin_unlock_irq(&pipe->rd_wait.lock);