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 <mina86@mina86.com>
Cc: Evgeniy Polyakov <zbr@ioremap.net>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index fa932c2..66efa96 100644 (file)
--- 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 45908e5..ca8081a 100644 (file)
--- 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;