[PATCH review for 4.9 49/50] drm/exynos: g2d: prevent integer overflow in

[PATCH review for 4.9 49/50] drm/exynos: g2d: prevent integer overflow in

[Levin, Alexander (Sasha Levin)]

From: Joonyoung Shim <jy0922.shim@samsung.com>

[ Upstream commit e41456bfc811f12b5dcda6f2d6849bdff68f6c0a ]

The size computations done in the ioctl function use an integer.
If userspace submits a request with req->cmd_nr or req->cmd_buf_nr
set to INT_MAX, the integer computations overflow later, leading
to potential (kernel) memory corruption.

Prevent this issue by enforcing a limit on the number of submitted
commands, so that we have enough headroom later for the size
computations.

Note that this change has no impact on the currently available
users in userspace, like e.g. libdrm/exynos.

While at it, also make a comment about the size computation more
detailed.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index fbd13fabdf2d..603d8425cca6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -1193,6 +1193,17 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
 	if (!node)
 		return -ENOMEM;
 
+	/*
+	 * To avoid an integer overflow for the later size computations, we
+	 * enforce a maximum number of submitted commands here. This limit is
+	 * sufficient for all conceivable usage cases of the G2D.
+	 */
+	if (req->cmd_nr > G2D_CMDLIST_DATA_NUM ||
+	    req->cmd_buf_nr > G2D_CMDLIST_DATA_NUM) {
+		dev_err(dev, "number of submitted G2D commands exceeds limit\n");
+		return -EINVAL;
+	}
+
 	node->event = NULL;
 
 	if (req->event_type != G2D_EVENT_NOT) {
@@ -1250,7 +1261,11 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
 		cmdlist->data[cmdlist->last++] = G2D_INTEN_ACF;
 	}
 
-	/* Check size of cmdlist: last 2 is about G2D_BITBLT_START */
+	/*
+	 * Check the size of cmdlist. The 2 that is added last comes from
+	 * the implicit G2D_BITBLT_START that is appended once we have
+	 * checked all the submitted commands.
+	 */
 	size = cmdlist->last + req->cmd_nr * 2 + req->cmd_buf_nr * 2 + 2;
 	if (size > G2D_CMDLIST_DATA_NUM) {
 		dev_err(dev, "cmdlist size is too big\n");
-- 
2.11.0

[PATCH review for 4.9 50/50] PCI: Avoid possible deadlock on pci_lock and p->pi_lock

[Levin, Alexander (Sasha Levin)]

From: Bjorn Helgaas <bhelgaas@google.com>

[ Upstream commit cdcb33f9824429a926b971bf041a6cec238f91ff ]

pci_lock is an IRQ-safe spinlock that protects all accesses to PCI
configuration space (see PCI_OP_READ() and PCI_OP_WRITE() in pci/access.c).

The pci_cfg_access_unlock() path acquires pci_lock, then p->pi_lock (inside
wake_up_all()).  According to lockdep, there is a possible path involving
snbep_uncore_pci_read_counter() that could acquire them in the reverse
order: acquiring p->pi_lock, then pci_lock, which could result in a
deadlock.  Lockdep details are in the bugzilla below.

Avoid the possible deadlock by dropping pci_lock before waking up any
config access waiters.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=192901
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 drivers/pci/access.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index d11cdbb8fba3..7b5cf6d1181a 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -672,8 +672,9 @@ void pci_cfg_access_unlock(struct pci_dev *dev)
 	WARN_ON(!dev->block_cfg_access);
 
 	dev->block_cfg_access = 0;
-	wake_up_all(&pci_cfg_wait);
 	raw_spin_unlock_irqrestore(&pci_lock, flags);
+
+	wake_up_all(&pci_cfg_wait);
 }
 EXPORT_SYMBOL_GPL(pci_cfg_access_unlock);
 
-- 
2.11.0