linux-kernel - Re: [PATCH -next] spi: Fix double IDR allocation with DT aliases

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2024176715.614.1535485625914.JavaMail.root@compulab.co.il>
Date:   Tue, 28 Aug 2018 22:47:05 +0300 (IDT)
From:   Kirill kapranov <kirill.kapranov@...pulab.co.il>
To:     Mark Brown <broonie@...nel.org>
Cc:     Geert Uytterhoeven <geert+renesas@...der.be>,
        linux-spi@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
        linux-kernel@...r.kernel.org, festevam@...il.com,
        linux@...ck-us.net
Subject: Re: [PATCH -next] spi: Fix double IDR allocation with DT aliases

> Right, that clearly wasn't an intended effect, though - should be using
> the max of the big constant and the maximum static ID.

Due to difficulties of enumeration of all device in a system, I propose
to assign safe sub-range for dynamic IDs in the upper-half of ID range the following way.

NOTE-0: This patch has to be applied after Geert's patch,
which fixes double call of idr_alloc. (above in the thread). Many thanks, Geert!

NOTE-1: The best solution, IMHO, would be migration to property_get_*
functions, declared in linux/property.h 

[PATCH] Eliminate possibility of conflict between static and dynamic IDs

For systems without DT support allocate dynamical bus ID in a safe sub-range
(if given), otherwise in upper half of the range so as to avoid possible
conflict between statically and dynamically allocated IDs.

Signed-off-by: Kirill Kapranov <kirill.kapranov@...pulab.co.il>
---
 drivers/spi/Kconfig |  8 ++++++++
 drivers/spi/spi.c   | 22 +++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 671d078349cc..7498fae0113b 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -47,6 +47,14 @@ config SPI_MASTER
 
 if SPI_MASTER
 
+config SPI_MASTER_DYN_BUS_NUM_FIRST
+	int "First dynamically assigned SPI bus ID" if EXPERT
+	default 0
+	help
+	  This value can be used as the beginning of sub-range for dynamic
+	  allocation of SPI bus ID in case of absence of DT. If -1 chosen, the
+	  sub-range will be allocated in upper half of the SPI bus ID range.
+
 config SPI_MEM
 	bool "SPI memory extension"
 	help
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9da0bc5a036c..3ac0cf0ab49c 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -49,6 +49,18 @@
 
 #include "internals.h"
 
+#ifndef CHAR_BIT
+#define CHAR_BIT 8	/* Normally in <limits.h> */
+#endif
+#if !defined(CONFIG_SPI_MASTER_DYN_BUS_NUM_FIRST) || \
+	CONFIG_SPI_MASTER_DYN_BUS_NUM_FIRST < 0
+//Half of the biggest signed int of this size
+#define SPI_DYN_FIRST_NUM (((1 << \
+	(sizeof(((struct spi_controller*)0)->bus_num) * CHAR_BIT - 1)) - 1) / 2)
+#else
+#define SPI_DYN_FIRST_NUM CONFIG_SPI_MASTER_DYN_BUS_NUM_FIRST
+#endif
+
 static DEFINE_IDR(spi_master_idr);
 
 static void spidev_release(struct device *dev)
@@ -2167,7 +2179,15 @@ int spi_register_controller(struct spi_controller *ctlr)
 	}
 	if (ctlr->bus_num < 0) {
 		first_dynamic = of_alias_get_highest_id("spi");
-		if (first_dynamic < 0)
+
+		if (first_dynamic == -ENOSYS)
+			/*
+			 * In case of system without DT support, allocate
+			 * dynamic bus ID in safe range, higher than the bound,
+			 * to avoid conflict between static and dynamic ID
+			 */
+			first_dynamic = SPI_DYN_FIRST_NUM;
+		else if (first_dynamic < 0)
 			first_dynamic = 0;
 		else
 			first_dynamic++;
-- 
2.11.0