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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Sun, 10 May 2020 01:58:50 +0200
From:   Akira Shimahara <akira215corp@...il.com>
To:     greg@...ah.com
Cc:     zbr@...emap.net, linux-kernel@...r.kernel.org,
        Akira Shimahara <akira215corp@...il.com>
Subject: [PATCH v5 2/9] w1_therm: fix reset_select_slave during discovery

Fix reset_select_slave issue during devices discovery by the master on
bus. The w1_reset_select_slave() from w1_io.c, which was previously used,
assume that if the slave count is 1 there is only one slave attached on
the bus. This is not always true. For example when discovering devices,
when the first device is discover by the bus master, its slave count is
1, but some other slaves may be on the bus.

In that case instead of adressing command to the attached slave the 
master throw a SKIP ROM command so that all slaves attached on the bus
will answer simultenaously causing data collision.

A dedicated reset_select_slave() function is implemented here,
it always perform an adressing to each slave using the MATCH ROM
command.

Signed-off-by: Akira Shimahara <akira215corp@...il.com>
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding <linux/string.h> include

 drivers/w1/slaves/w1_therm.c | 45 ++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index f7147b2..4dd139b 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/hwmon.h>
+#include <linux/string.h>
 
 #include <linux/w1.h>
 
@@ -92,6 +93,21 @@ struct therm_info {
 	u8 verdict;
 };
 
+/*---------------------Hardware Functions declaration-----------------------*/
+
+/**
+ * reset_select_slave() - reset and select a slave
+ * \brief Resets the bus and select the slave by sending a ROM MATCH cmd
+ * w1_reset_select_slave() from w1_io.c could not be used here because
+ * it sent a SKIP ROM command if only one device is on the line.
+ * At the beginning of the such process, sl->master->slave_count is 1 even if
+ * more devices are on the line, causing collision on the line.
+ * The w1 master lock must be held.
+ * \param sl the slave to select
+ * \return 0 if success, negative kernel error code otherwise
+ */
+static int reset_select_slave(struct w1_slave *sl);
+
 /*-----------------------Interface sysfs declaration------------------------*/
 
 /** \brief A callback function to output the temperature Old way
@@ -305,7 +321,7 @@ static inline int w1_DS18B20_precision(struct device *device, int val)
 	while (max_trying--) {
 		crc = 0;
 
-		if (!w1_reset_select_slave(sl)) {
+		if (!reset_select_slave(sl)) {
 			int count = 0;
 
 			/* read values to only alter precision bits */
@@ -318,7 +334,7 @@ static inline int w1_DS18B20_precision(struct device *device, int val)
 			if (rom[8] == crc) {
 				rom[4] = (rom[4] & ~mask) | (precision_bits & mask);
 
-				if (!w1_reset_select_slave(sl)) {
+				if (!reset_select_slave(sl)) {
 					w1_write_8(dev, W1_WRITE_SCRATCHPAD);
 					w1_write_8(dev, rom[2]);
 					w1_write_8(dev, rom[3]);
@@ -440,6 +456,21 @@ static void w1_therm_remove_slave(struct w1_slave *sl)
 
 /*---------------------------Hardware Functions-----------------------------*/
 
+/* Safe version of reser_select_slave - avoid using the one in w_io.c */
+static int reset_select_slave(struct w1_slave *sl)
+{
+	u8 match[9] = { W1_MATCH_ROM, };
+	u64 rn = le64_to_cpu(*((u64 *)&sl->reg_num));
+
+	if (w1_reset_bus(sl->master))
+		return -ENODEV;
+
+	memcpy(&match[1], &rn, 8);
+	w1_write_block(sl->master, match, 9);
+
+	return 0;
+}
+
 static ssize_t read_therm(struct device *device,
 			  struct w1_slave *sl, struct therm_info *info)
 {
@@ -467,7 +498,7 @@ static ssize_t read_therm(struct device *device,
 		info->verdict = 0;
 		info->crc = 0;
 
-		if (!w1_reset_select_slave(sl)) {
+		if (!reset_select_slave(sl)) {
 			int count = 0;
 			unsigned int tm = 750;
 			unsigned long sleep_rem;
@@ -475,7 +506,7 @@ static ssize_t read_therm(struct device *device,
 			w1_write_8(dev, W1_READ_PSUPPLY);
 			external_power = w1_read_8(dev);
 
-			if (w1_reset_select_slave(sl))
+			if (reset_select_slave(sl))
 				continue;
 
 			/* 750ms strong pullup (or delay) after the convert */
@@ -505,7 +536,7 @@ static ssize_t read_therm(struct device *device,
 				}
 			}
 
-			if (!w1_reset_select_slave(sl)) {
+			if (!reset_select_slave(sl)) {
 
 				w1_write_8(dev, W1_READ_SCRATCHPAD);
 				count = w1_read_block(dev, info->rom, 9);
@@ -557,7 +588,7 @@ static inline int w1_therm_eeprom(struct device *device)
 	memset(rom, 0, sizeof(rom));
 
 	while (max_trying--) {
-		if (!w1_reset_select_slave(sl)) {
+		if (!reset_select_slave(sl)) {
 			unsigned int tm = 10;
 			unsigned long sleep_rem;
 
@@ -565,7 +596,7 @@ static inline int w1_therm_eeprom(struct device *device)
 			w1_write_8(dev, W1_READ_PSUPPLY);
 			external_power = w1_read_8(dev);
 
-			if (w1_reset_select_slave(sl))
+			if (reset_select_slave(sl))
 				continue;
 
 			/* 10ms strong pullup/delay after the copy command */
-- 
2.26.2

Powered by blists - more mailing lists