staging: iio: accel: Use __be16 instead of u16

Hello Aybuke Ozdemir,

The patch 03c6eaa37ad7: "staging: iio: accel: Use __be16 instead of u16" from Sep 28, 2014, leads to the following static checker warning:

	drivers/staging/iio/accel/sca3000_ring.c:120 sca3000_read_first_n_hw_rb()
	warn: potential pointer math issue ('rx' is a 16 bit pointer)

drivers/staging/iio/accel/sca3000_ring.c
107 */
108 if (count > num_available * bytes_per_sample)

   109                  num_read = num_available*bytes_per_sample;
   110          else
   111                  num_read = count;
   112  
   113          ret = sca3000_read_data(st,
   114                                  SCA3000_REG_ADDR_RING_OUT,
   115                                  &rx, num_read);
   116          if (ret)
   117                  goto error_ret;
   118  
   119          for (i = 0; i < num_read; i++)
   120                  *(((u16 *)rx) + i) = be16_to_cpup((__be16 *)rx + i);
                          ^^^^^^^^^^^^^^^

We're writing beyond the end of the array here because of the pointer math issue. The fix is probably to say:

		for (i = 0; i < num_read / sizeof(u16); i++)
			*(((u16 *)rx) + i) = be16_to_cpup((__be16 *)rx + i);

   121  
   122          if (copy_to_user(buf, rx, num_read))
   123                  ret = -EFAULT;

regards,
dan carpenter

Hello Aybuke Ozdemir,

The patch 03c6eaa37ad7: "staging: iio: accel: Use __be16 instead of u16" from Sep 28, 2014, leads to the following static checker warning:

drivers/staging/iio/accel/sca3000_ring.c:120 sca3000_read_first_n_hw_rb()
warn: potential pointer math issue ('rx' is a 16 bit pointer)

drivers/staging/iio/accel/sca3000_ring.c
107 */
108 if (count > num_available * bytes_per_sample)

   109                  num_read = num_available*bytes_per_sample;
   110          else
   111                  num_read = count;
   112
   113          ret = sca3000_read_data(st,
   114                                  SCA3000_REG_ADDR_RING_OUT,
   115                                  &rx, num_read);
   116          if (ret)
   117                  goto error_ret;
   118
   119          for (i = 0; i < num_read; i++)
   120                  *(((u16 *)rx) + i) = be16_to_cpup((__be16 *)rx + i);
                          ^^^^^^^^^^^^^^^

We're writing beyond the end of the array here because of the pointer math issue. The fix is probably to say:

                for (i = 0; i < num_read / sizeof(u16); i++)
                        *(((u16 *)rx) + i) = be16_to_cpup((__be16 *)rx + i);

   121
   122          if (copy_to_user(buf, rx, num_read))
   123                  ret = -EFAULT;

Quoted 7 lines

   119          for (i = 0; i < num_read; i++)
   120                  *(((u16 *)rx) + i) = be16_to_cpup((__be16 *)rx + i);
                          ^^^^^^^^^^^^^^^

We're writing beyond the end of the array here because of the pointer math issue. The fix is probably to say:

Quoted 6 lines

                for (i = 0; i < num_read / sizeof(u16); i++)
                        *(((u16 *)rx) + i) = be16_to_cpup((__be16 *)rx + i);

   121
   122          if (copy_to_user(buf, rx, num_read))
   123                  ret = -EFAULT;

Looks good to me. Please send a formal patch.

Quoted 10 lines · Daniel Baluta

Quoted 10 lines

   119          for (i = 0; i < num_read; i++)
   120                  *(((u16 *)rx) + i) = be16_to_cpup((__be16 *)rx + i);
                          ^^^^^^^^^^^^^^^

We're writing beyond the end of the array here because of the pointer math issue. The fix is probably to say:

                for (i = 0; i < num_read / sizeof(u16); i++)
                        *(((u16 *)rx) + i) = be16_to_cpup((__be16 *)rx + i);

Quoted 5 lines

Quoted 3 lines

   121
   122          if (copy_to_user(buf, rx, num_read))
   123                  ret = -EFAULT;

Looks good to me. Please send a formal patch.

It's weird that no one has noticed this bug in testing because we end up corrupting memory every time this function is called.