From bb67093796a41e0f2601f5c0022fd8873ed59fee Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Tue, 12 Nov 2013 15:11:42 -0800 Subject: [PATCH] drivers: w1: make w1_slave::flags long to avoid memory corruption On architectures where long is more then 32 bits, modifying a 32-bit field with set_bit (and other atomic bit operations) may cause bytes following the field to by modified. Because the endianness of the bits within a field is the native endianness of the CPU[1], on big-endian machines, bit number zero is in the last byte of the field. Therefore, `set_bit(0, ptr)' on a 64-bit big-endian machine is roughly equivalent to `((char *)ptr)[7] |= 1', and since w1 driver uses a 32-bit field for holding the flags, this causes bytes beyond the field to be modified. [1] From Documentation/atomic_ops.txt: Native atomic bit operations are defined to operate on objects aligned to the size of an "unsigned long" C data type, and are least of that size. The endianness of the bits within each "unsigned long" are the native endianness of the cpu. Signed-off-by: Michal Nazarewicz Cc: Evgeniy Polyakov Cc: Greg KH Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/w1/w1.c | 10 +++++----- drivers/w1/w1.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index fa932c2f7d97..66efa96c4603 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -709,7 +709,7 @@ static int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn) sl->owner = THIS_MODULE; sl->master = dev; - set_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags); + set_bit(W1_SLAVE_ACTIVE, &sl->flags); memset(&msg, 0, sizeof(msg)); memcpy(&sl->reg_num, rn, sizeof(sl->reg_num)); @@ -866,7 +866,7 @@ void w1_slave_found(struct w1_master *dev, u64 rn) sl = w1_slave_search_device(dev, tmp); if (sl) { - set_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags); + set_bit(W1_SLAVE_ACTIVE, &sl->flags); } else { if (rn && tmp->crc == w1_calc_crc8((u8 *)&rn_le, 7)) w1_attach_slave_device(dev, tmp); @@ -984,14 +984,14 @@ void w1_search_process_cb(struct w1_master *dev, u8 search_type, struct w1_slave *sl, *sln; list_for_each_entry(sl, &dev->slist, w1_slave_entry) - clear_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags); + clear_bit(W1_SLAVE_ACTIVE, &sl->flags); w1_search_devices(dev, search_type, cb); list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) { - if (!test_bit(W1_SLAVE_ACTIVE, (unsigned long *)&sl->flags) && !--sl->ttl) + if (!test_bit(W1_SLAVE_ACTIVE, &sl->flags) && !--sl->ttl) w1_slave_detach(sl); - else if (test_bit(W1_SLAVE_ACTIVE, (unsigned long *)&sl->flags)) + else if (test_bit(W1_SLAVE_ACTIVE, &sl->flags)) sl->ttl = dev->slave_ttl; } diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h index 45908e56c2f8..ca8081a101d6 100644 --- a/drivers/w1/w1.h +++ b/drivers/w1/w1.h @@ -67,8 +67,8 @@ struct w1_slave struct w1_reg_num reg_num; atomic_t refcnt; u8 rom[9]; - u32 flags; int ttl; + unsigned long flags; struct w1_master *master; struct w1_family *family; -- 2.20.1