aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid Fries <david@fries.net>2008-10-16 01:04:51 -0400
committerLinus Torvalds <torvalds@linux-foundation.org>2008-10-16 14:21:50 -0400
commit347ba8a588c3e49f357291e5a1ac38a11d7e052d (patch)
treee3f67be1903df707e60116f841b8082da7facc7b
parent07e003417b88deac4b887c98f499fc3b01bc8df0 (diff)
W1: w1_therm fix user buffer overflow and cat
Fixed data reading bug by replacing binary attribute with device one. Switching the sysfs read from bin_attribute to device_attribute. The data is far under PAGE_SIZE so the binary interface isn't required. As the device_attribute interface will make one call to w1_therm_read per file open and buffer, the result is, the following problems go away. buffer overflow: Execute a short read on w1_slave and w1_therm_read_bin would still return the full string size worth of data clobbering the user space buffer when it returned. Switching to device_attribute avoids the buffer overflow problems. With the snprintf formatted output dealing with short reads without doing a conversion per read would have been difficult. bad behavior: `cat w1_slave` would cause two temperature conversions to take place. Previously the code assumed W1_SLAVE_DATA_SIZE would be returned with each read. It would not return 0 unless the offset was less than W1_SLAVE_DATA_SIZE. The result was the first read did a temperature conversion, filled the buffer and returned, the offset in the second read would be less than W1_SLAVE_DATA_SIZE and also fill the buffer and return, the third read would finnally have a big enough offset to return 0 and cause cat to stop. Now w1_therm_read will be called at most once per open. Signed-off-by: David Fries <david@fries.net> Signed-off-by: Evgeniy Polyakov <johnpol@2ka.mipt.ru> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--drivers/w1/slaves/w1_therm.c55
-rw-r--r--drivers/w1/w1.h1
2 files changed, 20 insertions, 36 deletions
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index e87f464a6fb0..7de99dfd11c8 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -50,26 +50,20 @@ static u8 bad_roms[][9] = {
50 {} 50 {}
51 }; 51 };
52 52
53static ssize_t w1_therm_read_bin(struct kobject *, struct bin_attribute *, 53static ssize_t w1_therm_read(struct device *device,
54 char *, loff_t, size_t); 54 struct device_attribute *attr, char *buf);
55 55
56static struct bin_attribute w1_therm_bin_attr = { 56static struct device_attribute w1_therm_attr =
57 .attr = { 57 __ATTR(w1_slave, S_IRUGO, w1_therm_read, NULL);
58 .name = "w1_slave",
59 .mode = S_IRUGO,
60 },
61 .size = W1_SLAVE_DATA_SIZE,
62 .read = w1_therm_read_bin,
63};
64 58
65static int w1_therm_add_slave(struct w1_slave *sl) 59static int w1_therm_add_slave(struct w1_slave *sl)
66{ 60{
67 return sysfs_create_bin_file(&sl->dev.kobj, &w1_therm_bin_attr); 61 return device_create_file(&sl->dev, &w1_therm_attr);
68} 62}
69 63
70static void w1_therm_remove_slave(struct w1_slave *sl) 64static void w1_therm_remove_slave(struct w1_slave *sl)
71{ 65{
72 sysfs_remove_bin_file(&sl->dev.kobj, &w1_therm_bin_attr); 66 device_remove_file(&sl->dev, &w1_therm_attr);
73} 67}
74 68
75static struct w1_family_ops w1_therm_fops = { 69static struct w1_family_ops w1_therm_fops = {
@@ -168,30 +162,19 @@ static int w1_therm_check_rom(u8 rom[9])
168 return 0; 162 return 0;
169} 163}
170 164
171static ssize_t w1_therm_read_bin(struct kobject *kobj, 165static ssize_t w1_therm_read(struct device *device,
172 struct bin_attribute *bin_attr, 166 struct device_attribute *attr, char *buf)
173 char *buf, loff_t off, size_t count)
174{ 167{
175 struct w1_slave *sl = kobj_to_w1_slave(kobj); 168 struct w1_slave *sl = dev_to_w1_slave(device);
176 struct w1_master *dev = sl->master; 169 struct w1_master *dev = sl->master;
177 u8 rom[9], crc, verdict; 170 u8 rom[9], crc, verdict;
178 int i, max_trying = 10; 171 int i, max_trying = 10;
172 ssize_t c = PAGE_SIZE;
179 173
180 mutex_lock(&sl->master->mutex); 174 mutex_lock(&sl->master->mutex);
181 175
182 if (off > W1_SLAVE_DATA_SIZE) {
183 count = 0;
184 goto out;
185 }
186 if (off + count > W1_SLAVE_DATA_SIZE) {
187 count = 0;
188 goto out;
189 }
190
191 memset(buf, 0, count);
192 memset(rom, 0, sizeof(rom)); 176 memset(rom, 0, sizeof(rom));
193 177
194 count = 0;
195 verdict = 0; 178 verdict = 0;
196 crc = 0; 179 crc = 0;
197 180
@@ -211,7 +194,9 @@ static ssize_t w1_therm_read_bin(struct kobject *kobj,
211 194
212 w1_write_8(dev, W1_READ_SCRATCHPAD); 195 w1_write_8(dev, W1_READ_SCRATCHPAD);
213 if ((count = w1_read_block(dev, rom, 9)) != 9) { 196 if ((count = w1_read_block(dev, rom, 9)) != 9) {
214 dev_warn(&dev->dev, "w1_read_block() returned %d instead of 9.\n", count); 197 dev_warn(device, "w1_read_block() "
198 "returned %u instead of 9.\n",
199 count);
215 } 200 }
216 201
217 crc = w1_calc_crc8(rom, 8); 202 crc = w1_calc_crc8(rom, 8);
@@ -226,22 +211,22 @@ static ssize_t w1_therm_read_bin(struct kobject *kobj,
226 } 211 }
227 212
228 for (i = 0; i < 9; ++i) 213 for (i = 0; i < 9; ++i)
229 count += sprintf(buf + count, "%02x ", rom[i]); 214 c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ", rom[i]);
230 count += sprintf(buf + count, ": crc=%02x %s\n", 215 c -= snprintf(buf + PAGE_SIZE - c, c, ": crc=%02x %s\n",
231 crc, (verdict) ? "YES" : "NO"); 216 crc, (verdict) ? "YES" : "NO");
232 if (verdict) 217 if (verdict)
233 memcpy(sl->rom, rom, sizeof(sl->rom)); 218 memcpy(sl->rom, rom, sizeof(sl->rom));
234 else 219 else
235 dev_warn(&dev->dev, "18S20 doesn't respond to CONVERT_TEMP.\n"); 220 dev_warn(device, "18S20 doesn't respond to CONVERT_TEMP.\n");
236 221
237 for (i = 0; i < 9; ++i) 222 for (i = 0; i < 9; ++i)
238 count += sprintf(buf + count, "%02x ", sl->rom[i]); 223 c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ", sl->rom[i]);
239 224
240 count += sprintf(buf + count, "t=%d\n", w1_convert_temp(rom, sl->family->fid)); 225 c -= snprintf(buf + PAGE_SIZE - c, c, "t=%d\n",
241out: 226 w1_convert_temp(rom, sl->family->fid));
242 mutex_unlock(&dev->mutex); 227 mutex_unlock(&dev->mutex);
243 228
244 return count; 229 return PAGE_SIZE - c;
245} 230}
246 231
247static int __init w1_therm_init(void) 232static int __init w1_therm_init(void)
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index 00b84ab22808..cdaa6fffbfc7 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -46,7 +46,6 @@ struct w1_reg_num
46#include "w1_family.h" 46#include "w1_family.h"
47 47
48#define W1_MAXNAMELEN 32 48#define W1_MAXNAMELEN 32
49#define W1_SLAVE_DATA_SIZE 128
50 49
51#define W1_SEARCH 0xF0 50#define W1_SEARCH 0xF0
52#define W1_ALARM_SEARCH 0xEC 51#define W1_ALARM_SEARCH 0xEC