linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin LaHaise <bcrl@kvack.org>
To: Dan Aloni <dan@kernelim.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	security@kernel.org, linux-aio@kvack.org,
	linux-kernel@vger.kernel.org, Mateusz Guzik <mguzik@redhat.com>,
	Petr Matousek <pmatouse@redhat.com>,
	Kent Overstreet <kmo@daterainc.com>,
	Jeff Moyer <jmoyer@redhat.com>,
	stable@vger.kernel.org
Subject: Re: Revert "aio: fix aio request leak when events are reaped by user space"
Date: Fri, 22 Aug 2014 12:26:30 -0400	[thread overview]
Message-ID: <20140822162630.GF20391@kvack.org> (raw)
In-Reply-To: <20140822161502.GA30392@gmail.com>

On Fri, Aug 22, 2014 at 07:15:02PM +0300, Dan Aloni wrote:
> Sorry, I was waiting for a new patch from your direction, I should
> have replied earlier. What bothered me about the patch you sent is that
> completed_events is added as a new field but nothing assigns to it, so I 
> wonder how it can be effective.

Ah, that was missing a hunk then.  Try this version instead.

		-ben
-- 
"Thought is the essence of where you are now."


diff --git a/fs/aio.c b/fs/aio.c
index ae63587..fbdcc47 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -142,6 +142,7 @@ struct kioctx {
 	struct {
 		unsigned	tail;
 		spinlock_t	completion_lock;
+		unsigned	completed_events;
 	} ____cacheline_aligned_in_smp;
 
 	struct page		*internal_pages[AIO_RING_PAGES];
@@ -857,6 +858,31 @@ out:
 	return ret;
 }
 
+static void refill_reqs_available(struct kioctx *ctx)
+{
+	spin_lock_irq(&ctx->completion_lock);
+	if (ctx->completed_events) {
+		unsigned head, tail, avail, completed;
+		struct aio_ring *ring;
+
+		ring = kmap_atomic(ctx->ring_pages[0]);
+		head = ACCESS_ONCE(ring->head);
+		tail = ACCESS_ONCE(ring->tail);
+		kunmap_atomic(ring);
+
+		avail = (head <= tail ?  tail : ctx->nr_events) - head;
+		completed = ctx->completed_events;
+		if (avail < completed)
+			completed -= avail;
+		else
+			completed = 0;
+		put_reqs_available(ctx, completed);
+	}
+
+	spin_unlock_irq(&ctx->completion_lock);
+}
+
+
 /* aio_get_req
  *	Allocate a slot for an aio request.
  * Returns NULL if no requests are free.
@@ -865,8 +891,11 @@ static inline struct kiocb *aio_get_req(struct kioctx *ctx)
 {
 	struct kiocb *req;
 
-	if (!get_reqs_available(ctx))
-		return NULL;
+	if (!get_reqs_available(ctx)) {
+		refill_reqs_available(ctx);
+		if (!get_reqs_available(ctx))
+			return NULL;
+	}
 
 	req = kmem_cache_alloc(kiocb_cachep, GFP_KERNEL|__GFP_ZERO);
 	if (unlikely(!req))
@@ -958,6 +987,7 @@ void aio_complete(struct kiocb *iocb, long res, long res2)
 	 */
 	spin_lock_irqsave(&ctx->completion_lock, flags);
 
+	ctx->completed_events++;
 	tail = ctx->tail;
 	pos = tail + AIO_EVENTS_OFFSET;
 
@@ -1005,7 +1035,6 @@ void aio_complete(struct kiocb *iocb, long res, long res2)
 
 	/* everything turned out well, dispose of the aiocb. */
 	kiocb_free(iocb);
-	put_reqs_available(ctx, 1);
 
 	/*
 	 * We have to order our ring_info tail store above and test

  reply	other threads:[~2014-08-22 16:26 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-24 18:01 [PATCH 0/2] aio: fixes for kernel memory disclosure in aio read events Benjamin LaHaise
2014-06-24 18:01 ` [PATCH 1/2] aio: fix aio request leak when events are reaped by userspace Benjamin LaHaise
2014-06-24 18:20   ` Jeff Moyer
2014-08-19 16:37   ` Revert "aio: fix aio request leak when events are reaped by user space" Dan Aloni
2014-08-19 16:54     ` Benjamin LaHaise
2014-08-19 17:14       ` Dan Aloni
2014-08-20  0:46         ` Benjamin LaHaise
2014-08-22 16:01           ` Benjamin LaHaise
2014-08-22 16:15             ` Dan Aloni
2014-08-22 16:26               ` Benjamin LaHaise [this message]
2014-08-22 18:51                 ` Dan Aloni
2014-08-22 21:43                   ` Linus Torvalds
2014-08-24 18:11                     ` Benjamin LaHaise
2014-08-26  1:11                     ` Kent Overstreet
2014-08-24 18:05                   ` Benjamin LaHaise
2014-08-24 18:48                     ` Dan Aloni
2014-08-27 20:26                       ` Jeff Moyer
2014-08-25 15:06                 ` Elliott, Robert (Server Storage)
2014-08-25 15:11                   ` Benjamin LaHaise
2014-06-24 18:02 ` [PATCH 2/2] aio: fix kernel memory disclosure in io_getevents() introduced in v3.10 Benjamin LaHaise
2014-06-24 18:23   ` Jeff Moyer
2014-06-24 18:39     ` Benjamin LaHaise
2014-06-24 19:21       ` Jeff Moyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140822162630.GF20391@kvack.org \
    --to=bcrl@kvack.org \
    --cc=dan@kernelim.com \
    --cc=jmoyer@redhat.com \
    --cc=kmo@daterainc.com \
    --cc=linux-aio@kvack.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mguzik@redhat.com \
    --cc=pmatouse@redhat.com \
    --cc=security@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).