Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
On Tue, Dec 4, 2018 at 11:26 PM Tony Battersby wrote: > > On 12/4/18 3:30 PM, Andy Shevchenko wrote: > > On Tue, Dec 4, 2018 at 10:18 PM Matthew Wilcox 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
On 12/4/18 3:30 PM, Andy Shevchenko wrote: > On Tue, Dec 4, 2018 at 10:18 PM Matthew Wilcox 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
On Tue, Dec 04, 2018 at 12:28:54PM -0800, Andrew Morton wrote: > On Tue, 4 Dec 2018 12:18:01 -0800 Matthew Wilcox 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. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
On Tue, Dec 4, 2018 at 10:18 PM Matthew Wilcox 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
On Tue, 4 Dec 2018 12:18:01 -0800 Matthew Wilcox 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 > > 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 > > > > 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 > > > > > > > > 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. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
On Tue, Dec 04, 2018 at 12:14:43PM -0800, Andrew Morton wrote: > On Tue, 4 Dec 2018 11:22:34 -0500 Tony Battersby > 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 > > > 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 > > > > > > 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. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
On Tue, 4 Dec 2018 11:22:34 -0500 Tony Battersby 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 > > 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 > > > > 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? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
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 > 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 > > 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
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 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 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. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 9/9] dmapool: debug: prevent endless loop in case of corruption
Prevent a possible endless loop with DMAPOOL_DEBUG enabled if a buggy driver corrupts DMA pool memory. Signed-off-by: Tony Battersby --- --- linux/mm/dmapool.c.orig 2018-08-06 17:52:53.0 -0400 +++ linux/mm/dmapool.c 2018-08-06 17:53:31.0 -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(>lock, flags); + dev_err(pool->dev, + "dma_pool_free %s, dma %pad already free\n", + pool->name, ); + 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(>lock, flags); + dev_err(pool->dev, + "dma_pool_free %s, freelist corrupted\n", + pool->name); + return; } - spin_unlock_irqrestore(>lock, flags); - dev_err(pool->dev, - "dma_pool_free %s, dma %pad already free\n", - pool->name, ); - 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu