[PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Tony Battersby]

Prevent a possible endless loop with DMAPOOL_DEBUG enabled if a buggy
driver corrupts DMA pool memory.

Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
---
--- linux/mm/dmapool.c.orig	2018-08-06 17:52:53.000000000 -0400
+++ linux/mm/dmapool.c	2018-08-06 17:53:31.000000000 -0400
@@ -454,17 +454,39 @@ void dma_pool_free(struct dma_pool *pool
 	{
 		void *page_vaddr = vaddr - offset;
 		unsigned int chain = page->dma_free_off;
+		unsigned int free_blks = 0;
+
 		while (chain < pool->allocation) {
-			if (chain != offset) {
-				chain = *(int *)(page_vaddr + chain);
-				continue;
+			if (unlikely(chain == offset)) {
+				spin_unlock_irqrestore(&pool->lock, flags);
+				dev_err(pool->dev,
+					"dma_pool_free %s, dma %pad already free\n",
+					pool->name, &dma);
+				return;
+			}
+
+			/*
+			 * A buggy driver could corrupt the freelist by
+			 * use-after-free, buffer overflow, etc.  Besides
+			 * checking for corruption, this also prevents an
+			 * endless loop in case corruption causes a circular
+			 * loop in the freelist.
+			 */
+			if (unlikely(++free_blks + page->dma_in_use >
+				     pool->blks_per_alloc)) {
+ freelist_corrupt:
+				spin_unlock_irqrestore(&pool->lock, flags);
+				dev_err(pool->dev,
+					"dma_pool_free %s, freelist corrupted\n",
+					pool->name);
+				return;
 			}
-			spin_unlock_irqrestore(&pool->lock, flags);
-			dev_err(pool->dev,
-				"dma_pool_free %s, dma %pad already free\n",
-				pool->name, &dma);
-			return;
+
+			chain = *(int *)(page_vaddr + chain);
 		}
+		if (unlikely(free_blks + page->dma_in_use !=
+			     pool->blks_per_alloc))
+			goto freelist_corrupt;
 	}
 	memset(vaddr, POOL_POISON_FREED, pool->size);
 #endif

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Matthew Wilcox]

On Mon, Nov 12, 2018 at 10:46:35AM -0500, Tony Battersby wrote:
> Prevent a possible endless loop with DMAPOOL_DEBUG enabled if a buggy
> driver corrupts DMA pool memory.
> 
> Signed-off-by: Tony Battersby <tonyb@cybernetics.com>

I like it!  Also, here you're using blks_per_alloc in a way which isn't
normally in the performance path, but might be with the right config
options.  With that, I withdraw my objection to the previous patch and

Acked-by: Matthew Wilcox <willy@infradead.org>

Andrew, can you funnel these in through your tree?  If you'd rather not,
I don't mind stuffing them into a git tree and asking Linus to pull
for 4.21.

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Tony Battersby]

On 11/13/18 1:36 AM, Matthew Wilcox wrote:
> On Mon, Nov 12, 2018 at 10:46:35AM -0500, Tony Battersby wrote:
>> Prevent a possible endless loop with DMAPOOL_DEBUG enabled if a buggy
>> driver corrupts DMA pool memory.
>>
>> Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
> I like it!  Also, here you're using blks_per_alloc in a way which isn't
> normally in the performance path, but might be with the right config
> options.  With that, I withdraw my objection to the previous patch and
>
> Acked-by: Matthew Wilcox <willy@infradead.org>
>
> Andrew, can you funnel these in through your tree?  If you'd rather not,
> I don't mind stuffing them into a git tree and asking Linus to pull
> for 4.21.
>
No reply for 3 weeks, so adding Andrew Morton to recipient list.

Andrew, I have 9 dmapool patches ready for merging in 4.21.  See Matthew
Wilcox's request above.

Tony Battersby

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Andrew Morton]

On Tue, 4 Dec 2018 11:22:34 -0500 Tony Battersby <tonyb@cybernetics.com> wrote:

> On 11/13/18 1:36 AM, Matthew Wilcox wrote:
> > On Mon, Nov 12, 2018 at 10:46:35AM -0500, Tony Battersby wrote:
> >> Prevent a possible endless loop with DMAPOOL_DEBUG enabled if a buggy
> >> driver corrupts DMA pool memory.
> >>
> >> Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
> > I like it!  Also, here you're using blks_per_alloc in a way which isn't
> > normally in the performance path, but might be with the right config
> > options.  With that, I withdraw my objection to the previous patch and
> >
> > Acked-by: Matthew Wilcox <willy@infradead.org>
> >
> > Andrew, can you funnel these in through your tree?  If you'd rather not,
> > I don't mind stuffing them into a git tree and asking Linus to pull
> > for 4.21.
> >
> No reply for 3 weeks, so adding Andrew Morton to recipient list.
> 
> Andrew, I have 9 dmapool patches ready for merging in 4.21.  See Matthew
> Wilcox's request above.
> 

I'll take a look, but I see that this v4 series has several review
comments from Matthew which remain unresponded to.  Please attend to
that.

Also, Andy had issues with the v2 series so it would be good to hear an
update from him?

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Matthew Wilcox]

On Tue, Dec 04, 2018 at 12:14:43PM -0800, Andrew Morton wrote:
> On Tue, 4 Dec 2018 11:22:34 -0500 Tony Battersby <tonyb@cybernetics.com> wrote:
> 
> > On 11/13/18 1:36 AM, Matthew Wilcox wrote:
> > > On Mon, Nov 12, 2018 at 10:46:35AM -0500, Tony Battersby wrote:
> > >> Prevent a possible endless loop with DMAPOOL_DEBUG enabled if a buggy
> > >> driver corrupts DMA pool memory.
> > >>
> > >> Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
> > > I like it!  Also, here you're using blks_per_alloc in a way which isn't
> > > normally in the performance path, but might be with the right config
> > > options.  With that, I withdraw my objection to the previous patch and
> > >
> > > Acked-by: Matthew Wilcox <willy@infradead.org>
> > >
> > > Andrew, can you funnel these in through your tree?  If you'd rather not,
> > > I don't mind stuffing them into a git tree and asking Linus to pull
> > > for 4.21.
> > >
> > No reply for 3 weeks, so adding Andrew Morton to recipient list.
> > 
> > Andrew, I have 9 dmapool patches ready for merging in 4.21.� See Matthew
> > Wilcox's request above.
> > 
> 
> I'll take a look, but I see that this v4 series has several review
> comments from Matthew which remain unresponded to.  Please attend to
> that.

I only had a review comment on 8/9, which I then withdrew during my review
of patch 9/9.  Unless I missed something during my re-review of my responses?

> Also, Andy had issues with the v2 series so it would be good to hear an
> update from him?

Certainly.

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Andrew Morton]

On Tue, 4 Dec 2018 12:18:01 -0800 Matthew Wilcox <willy@infradead.org> wrote:

> On Tue, Dec 04, 2018 at 12:14:43PM -0800, Andrew Morton wrote:
> > On Tue, 4 Dec 2018 11:22:34 -0500 Tony Battersby <tonyb@cybernetics.com> wrote:
> > 
> > > On 11/13/18 1:36 AM, Matthew Wilcox wrote:
> > > > On Mon, Nov 12, 2018 at 10:46:35AM -0500, Tony Battersby wrote:
> > > >> Prevent a possible endless loop with DMAPOOL_DEBUG enabled if a buggy
> > > >> driver corrupts DMA pool memory.
> > > >>
> > > >> Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
> > > > I like it!  Also, here you're using blks_per_alloc in a way which isn't
> > > > normally in the performance path, but might be with the right config
> > > > options.  With that, I withdraw my objection to the previous patch and
> > > >
> > > > Acked-by: Matthew Wilcox <willy@infradead.org>
> > > >
> > > > Andrew, can you funnel these in through your tree?  If you'd rather not,
> > > > I don't mind stuffing them into a git tree and asking Linus to pull
> > > > for 4.21.
> > > >
> > > No reply for 3 weeks, so adding Andrew Morton to recipient list.
> > > 
> > > Andrew, I have 9 dmapool patches ready for merging in 4.21.  See Matthew
> > > Wilcox's request above.
> > > 
> > 
> > I'll take a look, but I see that this v4 series has several review
> > comments from Matthew which remain unresponded to.  Please attend to
> > that.
> 
> I only had a review comment on 8/9, which I then withdrew during my review
> of patch 9/9.  Unless I missed something during my re-review of my responses?

And in 0/9, that 1.3MB allocation.

Maybe it's using kvmalloc, I didn't look.

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Andy Shevchenko]

On Tue, Dec 4, 2018 at 10:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Tue, Dec 04, 2018 at 12:14:43PM -0800, Andrew Morton wrote:

> > Also, Andy had issues with the v2 series so it would be good to hear an
> > update from him?
>
> Certainly.

Hmm... I certainly forgot what was long time ago.
If I _was_ in Cc list and didn't comment, I'm fine with it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Matthew Wilcox]

On Tue, Dec 04, 2018 at 12:28:54PM -0800, Andrew Morton wrote:
> On Tue, 4 Dec 2018 12:18:01 -0800 Matthew Wilcox <willy@infradead.org> wrote:
> > I only had a review comment on 8/9, which I then withdrew during my review
> > of patch 9/9.  Unless I missed something during my re-review of my responses?
> 
> And in 0/9, that 1.3MB allocation.
> 
> Maybe it's using kvmalloc, I didn't look.

Oh!  That's the mptsas driver doing something utterly awful.  Not the
fault of this patchset, in any way.

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Tony Battersby]

On 12/4/18 3:30 PM, Andy Shevchenko wrote:
> On Tue, Dec 4, 2018 at 10:18 PM Matthew Wilcox <willy@infradead.org> wrote:
>> On Tue, Dec 04, 2018 at 12:14:43PM -0800, Andrew Morton wrote:
>>> Also, Andy had issues with the v2 series so it would be good to hear an
>>> update from him?
>> Certainly.
> Hmm... I certainly forgot what was long time ago.
> If I _was_ in Cc list and didn't comment, I'm fine with it.
>
v4 of the patchset is the same as v3 but with the last patch dropped. 
Andy had only one minor comment on v3 about the use of division in patch
#8, to which I replied.  That was back on August 8.

Tony

Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption

[Andy Shevchenko]

On Tue, Dec 4, 2018 at 11:26 PM Tony Battersby <tonyb@cybernetics.com> wrote:
>
> On 12/4/18 3:30 PM, Andy Shevchenko wrote:
> > On Tue, Dec 4, 2018 at 10:18 PM Matthew Wilcox <willy@infradead.org> wrote:
> >> On Tue, Dec 04, 2018 at 12:14:43PM -0800, Andrew Morton wrote:
> >>> Also, Andy had issues with the v2 series so it would be good to hear an
> >>> update from him?
> >> Certainly.
> > Hmm... I certainly forgot what was long time ago.
> > If I _was_ in Cc list and didn't comment, I'm fine with it.
> >
> v4 of the patchset is the same as v3 but with the last patch dropped.
> Andy had only one minor comment on v3 about the use of division in patch
> #8, to which I replied.  That was back on August 8.

Seems I'm fine with the last version then.

-- 
With Best Regards,
Andy Shevchenko