From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 823A315808B for ; Mon, 28 Mar 2022 22:50:28 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id AFFA9E0903; Mon, 28 Mar 2022 22:50:27 +0000 (UTC) Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 63E3BE0903 for ; Mon, 28 Mar 2022 22:50:27 +0000 (UTC) Received: from oystercatcher.gentoo.org (oystercatcher.gentoo.org [148.251.78.52]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id 49B7E341417 for ; Mon, 28 Mar 2022 22:50:26 +0000 (UTC) Received: from localhost.localdomain (localhost [IPv6:::1]) by oystercatcher.gentoo.org (Postfix) with ESMTP id 73293290 for ; Mon, 28 Mar 2022 22:50:24 +0000 (UTC) From: "Mike Pagano" To: gentoo-commits@lists.gentoo.org Content-Transfer-Encoding: 8bit Content-type: text/plain; charset=UTF-8 Reply-To: gentoo-dev@lists.gentoo.org, "Mike Pagano" Message-ID: <1648507811.20417eb334c61341e2228a680488aebe06bbcd16.mpagano@gentoo> Subject: [gentoo-commits] proj/linux-patches:5.15 commit in: / X-VCS-Repository: proj/linux-patches X-VCS-Files: 0000_README 2410_revert-swiotlb-rework-fix-info-leak-with-dma_from_device.patch X-VCS-Directories: / X-VCS-Committer: mpagano X-VCS-Committer-Name: Mike Pagano X-VCS-Revision: 20417eb334c61341e2228a680488aebe06bbcd16 X-VCS-Branch: 5.15 Date: Mon, 28 Mar 2022 22:50:24 +0000 (UTC) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-commits@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply X-Archives-Salt: 094f19b1-bae3-4bd9-b9ac-4bc37e41e8fd X-Archives-Hash: cf152b6f80ebc12942070b0fe72a36e4 commit: 20417eb334c61341e2228a680488aebe06bbcd16 Author: Mike Pagano gentoo org> AuthorDate: Mon Mar 28 22:50:11 2022 +0000 Commit: Mike Pagano gentoo org> CommitDate: Mon Mar 28 22:50:11 2022 +0000 URL: https://gitweb.gentoo.org/proj/linux-patches.git/commit/?id=20417eb3 Revert swiotlb: rework fix info leak with DMA_FROM_DEVICE Signed-off-by: Mike Pagano gentoo.org> 0000_README | 4 + ...rework-fix-info-leak-with-dma_from_device.patch | 187 +++++++++++++++++++++ 2 files changed, 191 insertions(+) diff --git a/0000_README b/0000_README index c59e90bb..0aae2d47 100644 --- a/0000_README +++ b/0000_README @@ -183,6 +183,10 @@ Patch: 2000_BT-Check-key-sizes-only-if-Secure-Simple-Pairing-enabled.patch From: https://lore.kernel.org/linux-bluetooth/20190522070540.48895-1-marcel@holtmann.org/raw Desc: Bluetooth: Check key sizes only when Secure Simple Pairing is enabled. See bug #686758 +Patch: 2410_revert-swiotlb-rework-fix-info-leak-with-dma_from_device.patch +From: https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git +Desc: Revert swiotlb: rework fix info leak with DMA_FROM_DEVICE + Patch: 2900_tmp513-Fix-build-issue-by-selecting-CONFIG_REG.patch From: https://bugs.gentoo.org/710790 Desc: tmp513 requies REGMAP_I2C to build. Select it by default in Kconfig. See bug #710790. Thanks to Phil Stracchino diff --git a/2410_revert-swiotlb-rework-fix-info-leak-with-dma_from_device.patch b/2410_revert-swiotlb-rework-fix-info-leak-with-dma_from_device.patch new file mode 100644 index 00000000..69476ab1 --- /dev/null +++ b/2410_revert-swiotlb-rework-fix-info-leak-with-dma_from_device.patch @@ -0,0 +1,187 @@ +From bddac7c1e02ba47f0570e494c9289acea3062cc1 Mon Sep 17 00:00:00 2001 +From: Linus Torvalds +Date: Sat, 26 Mar 2022 10:42:04 -0700 +Subject: Revert "swiotlb: rework "fix info leak with DMA_FROM_DEVICE"" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Linus Torvalds + +commit bddac7c1e02ba47f0570e494c9289acea3062cc1 upstream. + +This reverts commit aa6f8dcbab473f3a3c7454b74caa46d36cdc5d13. + +It turns out this breaks at least the ath9k wireless driver, and +possibly others. + +What the ath9k driver does on packet receive is to set up the DMA +transfer with: + + int ath_rx_init(..) + .. + bf->bf_buf_addr = dma_map_single(sc->dev, skb->data, + common->rx_bufsize, + DMA_FROM_DEVICE); + +and then the receive logic (through ath_rx_tasklet()) will fetch +incoming packets + + static bool ath_edma_get_buffers(..) + .. + dma_sync_single_for_cpu(sc->dev, bf->bf_buf_addr, + common->rx_bufsize, DMA_FROM_DEVICE); + + ret = ath9k_hw_process_rxdesc_edma(ah, rs, skb->data); + if (ret == -EINPROGRESS) { + /*let device gain the buffer again*/ + dma_sync_single_for_device(sc->dev, bf->bf_buf_addr, + common->rx_bufsize, DMA_FROM_DEVICE); + return false; + } + +and it's worth noting how that first DMA sync: + + dma_sync_single_for_cpu(..DMA_FROM_DEVICE); + +is there to make sure the CPU can read the DMA buffer (possibly by +copying it from the bounce buffer area, or by doing some cache flush). +The iommu correctly turns that into a "copy from bounce bufer" so that +the driver can look at the state of the packets. + +In the meantime, the device may continue to write to the DMA buffer, but +we at least have a snapshot of the state due to that first DMA sync. + +But that _second_ DMA sync: + + dma_sync_single_for_device(..DMA_FROM_DEVICE); + +is telling the DMA mapping that the CPU wasn't interested in the area +because the packet wasn't there. In the case of a DMA bounce buffer, +that is a no-op. + +Note how it's not a sync for the CPU (the "for_device()" part), and it's +not a sync for data written by the CPU (the "DMA_FROM_DEVICE" part). + +Or rather, it _should_ be a no-op. That's what commit aa6f8dcbab47 +broke: it made the code bounce the buffer unconditionally, and changed +the DMA_FROM_DEVICE to just unconditionally and illogically be +DMA_TO_DEVICE. + +[ Side note: purely within the confines of the swiotlb driver it wasn't + entirely illogical: The reason it did that odd DMA_FROM_DEVICE -> + DMA_TO_DEVICE conversion thing is because inside the swiotlb driver, + it uses just a swiotlb_bounce() helper that doesn't care about the + whole distinction of who the sync is for - only which direction to + bounce. + + So it took the "sync for device" to mean that the CPU must have been + the one writing, and thought it meant DMA_TO_DEVICE. ] + +Also note how the commentary in that commit was wrong, probably due to +that whole confusion, claiming that the commit makes the swiotlb code + + "bounce unconditionally (that is, also + when dir == DMA_TO_DEVICE) in order do avoid synchronising back stale + data from the swiotlb buffer" + +which is nonsensical for two reasons: + + - that "also when dir == DMA_TO_DEVICE" is nonsensical, as that was + exactly when it always did - and should do - the bounce. + + - since this is a sync for the device (not for the CPU), we're clearly + fundamentally not coping back stale data from the bounce buffers at + all, because we'd be copying *to* the bounce buffers. + +So that commit was just very confused. It confused the direction of the +synchronization (to the device, not the cpu) with the direction of the +DMA (from the device). + +Reported-and-bisected-by: Oleksandr Natalenko +Reported-by: Olha Cherevyk +Cc: Halil Pasic +Cc: Christoph Hellwig +Cc: Kalle Valo +Cc: Robin Murphy +Cc: Toke Høiland-Jørgensen +Cc: Maxime Bizon +Cc: Johannes Berg +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + Documentation/core-api/dma-attributes.rst | 8 ++++++++ + include/linux/dma-mapping.h | 8 ++++++++ + kernel/dma/swiotlb.c | 23 ++++++++--------------- + 3 files changed, 24 insertions(+), 15 deletions(-) + +--- a/Documentation/core-api/dma-attributes.rst ++++ b/Documentation/core-api/dma-attributes.rst +@@ -130,3 +130,11 @@ accesses to DMA buffers in both privileg + subsystem that the buffer is fully accessible at the elevated privilege + level (and ideally inaccessible or at least read-only at the + lesser-privileged levels). ++ ++DMA_ATTR_OVERWRITE ++------------------ ++ ++This is a hint to the DMA-mapping subsystem that the device is expected to ++overwrite the entire mapped size, thus the caller does not require any of the ++previous buffer contents to be preserved. This allows bounce-buffering ++implementations to optimise DMA_FROM_DEVICE transfers. +--- a/include/linux/dma-mapping.h ++++ b/include/linux/dma-mapping.h +@@ -62,6 +62,14 @@ + #define DMA_ATTR_PRIVILEGED (1UL << 9) + + /* ++ * This is a hint to the DMA-mapping subsystem that the device is expected ++ * to overwrite the entire mapped size, thus the caller does not require any ++ * of the previous buffer contents to be preserved. This allows ++ * bounce-buffering implementations to optimise DMA_FROM_DEVICE transfers. ++ */ ++#define DMA_ATTR_OVERWRITE (1UL << 10) ++ ++/* + * A dma_addr_t can hold any valid DMA or bus address for the platform. It can + * be given to a device to use as a DMA source or target. It is specific to a + * given device and there may be a translation between the CPU physical address +--- a/kernel/dma/swiotlb.c ++++ b/kernel/dma/swiotlb.c +@@ -627,14 +627,10 @@ phys_addr_t swiotlb_tbl_map_single(struc + for (i = 0; i < nr_slots(alloc_size + offset); i++) + mem->slots[index + i].orig_addr = slot_addr(orig_addr, i); + tlb_addr = slot_addr(mem->start, index) + offset; +- /* +- * When dir == DMA_FROM_DEVICE we could omit the copy from the orig +- * to the tlb buffer, if we knew for sure the device will +- * overwirte the entire current content. But we don't. Thus +- * unconditional bounce may prevent leaking swiotlb content (i.e. +- * kernel memory) to user-space. +- */ +- swiotlb_bounce(dev, tlb_addr, mapping_size, DMA_TO_DEVICE); ++ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && ++ (!(attrs & DMA_ATTR_OVERWRITE) || dir == DMA_TO_DEVICE || ++ dir == DMA_BIDIRECTIONAL)) ++ swiotlb_bounce(dev, tlb_addr, mapping_size, DMA_TO_DEVICE); + return tlb_addr; + } + +@@ -701,13 +697,10 @@ void swiotlb_tbl_unmap_single(struct dev + void swiotlb_sync_single_for_device(struct device *dev, phys_addr_t tlb_addr, + size_t size, enum dma_data_direction dir) + { +- /* +- * Unconditional bounce is necessary to avoid corruption on +- * sync_*_for_cpu or dma_ummap_* when the device didn't overwrite +- * the whole lengt of the bounce buffer. +- */ +- swiotlb_bounce(dev, tlb_addr, size, DMA_TO_DEVICE); +- BUG_ON(!valid_dma_direction(dir)); ++ if (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL) ++ swiotlb_bounce(dev, tlb_addr, size, DMA_TO_DEVICE); ++ else ++ BUG_ON(dir != DMA_FROM_DEVICE); + } + + void swiotlb_sync_single_for_cpu(struct device *dev, phys_addr_t tlb_addr,