Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

dma-buf: Add debug option

We have too many people abusing the struct page they can get at but
really shouldn't in importers. Aside from that the backing page might
simply not exist (for dynamic p2p mappings) looking at it and using it
e.g. for mmap can also wreak the page handling of the exporter
completely. Importers really must go through the proper interface like
dma_buf_mmap for everything.

I'm semi-tempted to enforce this for dynamic importers since those
really have no excuse at all to break the rules.

Unfortuantely we can't store the right pointers somewhere safe to make
sure we oops on something recognizable, so best is to just wrangle
them a bit by flipping all the bits. At least on x86 kernel addresses
have all their high bits sets and the struct page array is fairly low
in the kernel mapping, so flipping all the bits gives us a very high
pointer in userspace and hence excellent chances for an invalid
dereference.

v2: Add a note to the @map_dma_buf hook that exporters shouldn't do
fancy caching tricks, which would blow up with this address scrambling
trick here (Chris)

Enable by default when CONFIG_DMA_API_DEBUG is enabled.

v3: Only one copy of the mangle/unmangle code (Christian)

v4: #ifdef, not #if (0day)

v5: sg_table can also be an ERR_PTR (Chris, Christian)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: David Stevens <stevensd@chromium.org>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Link: https://patchwork.freedesktop.org/patch/msgid/20210115164739.3958206-1-daniel.vetter@ffwll.ch

+56 -4
+8
drivers/dma-buf/Kconfig
··· 50 50 This is marked experimental because we don't yet have a consistent 51 51 execution context and memory management between drivers. 52 52 53 + config DMABUF_DEBUG 54 + bool "DMA-BUF debug checks" 55 + default y if DMA_API_DEBUG 56 + help 57 + This option enables additional checks for DMA-BUF importers and 58 + exporters. Specifically it validates that importers do not peek at the 59 + underlying struct page when they import a buffer. 60 + 53 61 config DMABUF_SELFTESTS 54 62 tristate "Selftests for the dma-buf interfaces" 55 63 default n
+42 -4
drivers/dma-buf/dma-buf.c
··· 653 653 } 654 654 EXPORT_SYMBOL_GPL(dma_buf_put); 655 655 656 + static void mangle_sg_table(struct sg_table *sg_table) 657 + { 658 + #ifdef CONFIG_DMABUF_DEBUG 659 + int i; 660 + struct scatterlist *sg; 661 + 662 + /* To catch abuse of the underlying struct page by importers mix 663 + * up the bits, but take care to preserve the low SG_ bits to 664 + * not corrupt the sgt. The mixing is undone in __unmap_dma_buf 665 + * before passing the sgt back to the exporter. */ 666 + for_each_sgtable_sg(sg_table, sg, i) 667 + sg->page_link ^= ~0xffUL; 668 + #endif 669 + 670 + } 671 + static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, 672 + enum dma_data_direction direction) 673 + { 674 + struct sg_table *sg_table; 675 + 676 + sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); 677 + 678 + if (!IS_ERR_OR_NULL(sg_table)) 679 + mangle_sg_table(sg_table); 680 + 681 + return sg_table; 682 + } 683 + 656 684 /** 657 685 * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list 658 686 * @dmabuf: [in] buffer to attach device to. ··· 752 724 goto err_unlock; 753 725 } 754 726 755 - sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL); 727 + sgt = __map_dma_buf(attach, DMA_BIDIRECTIONAL); 756 728 if (!sgt) 757 729 sgt = ERR_PTR(-ENOMEM); 758 730 if (IS_ERR(sgt)) { ··· 799 771 } 800 772 EXPORT_SYMBOL_GPL(dma_buf_attach); 801 773 774 + static void __unmap_dma_buf(struct dma_buf_attachment *attach, 775 + struct sg_table *sg_table, 776 + enum dma_data_direction direction) 777 + { 778 + /* uses XOR, hence this unmangles */ 779 + mangle_sg_table(sg_table); 780 + 781 + attach->dmabuf->ops->unmap_dma_buf(attach, sg_table, direction); 782 + } 783 + 802 784 /** 803 785 * dma_buf_detach - Remove the given attachment from dmabuf's attachments list 804 786 * @dmabuf: [in] buffer to detach from. ··· 827 789 if (dma_buf_is_dynamic(attach->dmabuf)) 828 790 dma_resv_lock(attach->dmabuf->resv, NULL); 829 791 830 - dmabuf->ops->unmap_dma_buf(attach, attach->sgt, attach->dir); 792 + __unmap_dma_buf(attach, attach->sgt, attach->dir); 831 793 832 794 if (dma_buf_is_dynamic(attach->dmabuf)) { 833 795 dma_buf_unpin(attach); ··· 949 911 } 950 912 } 951 913 952 - sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); 914 + sg_table = __map_dma_buf(attach, direction); 953 915 if (!sg_table) 954 916 sg_table = ERR_PTR(-ENOMEM); 955 917 ··· 1012 974 if (dma_buf_is_dynamic(attach->dmabuf)) 1013 975 dma_resv_assert_held(attach->dmabuf->resv); 1014 976 1015 - attach->dmabuf->ops->unmap_dma_buf(attach, sg_table, direction); 977 + __unmap_dma_buf(attach, sg_table, direction); 1016 978 1017 979 if (dma_buf_is_dynamic(attach->dmabuf) && 1018 980 !IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY))
+6
include/linux/dma-buf.h
··· 154 154 * On failure, returns a negative error value wrapped into a pointer. 155 155 * May also return -EINTR when a signal was received while being 156 156 * blocked. 157 + * 158 + * Note that exporters should not try to cache the scatter list, or 159 + * return the same one for multiple calls. Caching is done either by the 160 + * DMA-BUF code (for non-dynamic importers) or the importer. Ownership 161 + * of the scatter list is transferred to the caller, and returned by 162 + * @unmap_dma_buf. 157 163 */ 158 164 struct sg_table * (*map_dma_buf)(struct dma_buf_attachment *, 159 165 enum dma_data_direction);