* [bug report] iio: accel: bma400: Add support for single and double tap events
@ 2022-09-15 14:27 Dan Carpenter
2022-09-17 13:30 ` Jonathan Cameron
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-09-15 14:27 UTC (permalink / raw)
To: jagathjog1996, Alexander Potapenko; +Cc: linux-iio
Hello Jagath Jog J,
The patch 961db2da159d: "iio: accel: bma400: Add support for single
and double tap events" from Aug 31, 2022, leads to the following
Smatch static checker warning:
drivers/iio/accel/bma400_core.c:1287 bma400_tap_event_en()
error: uninitialized symbol 'field_value'.
drivers/iio/accel/bma400_core.c
1242 static int bma400_tap_event_en(struct bma400_data *data,
1243 enum iio_event_direction dir, int state)
1244 {
1245 unsigned int mask, field_value;
^^^^^^^^^^^^
This is uninitialized.
1246 int ret;
1247
1248 /*
1249 * Tap interrupts can be configured only in normal mode.
1250 * See table in section 4.3 "Power modes - performance modes" of
1251 * datasheet v1.2.
1252 */
1253 if (data->power_mode != POWER_MODE_NORMAL)
1254 return -EINVAL;
1255
1256 /*
1257 * Tap interrupts are operating with a data rate of 200Hz.
1258 * See section 4.7 "Tap sensing interrupt" in datasheet v1.2.
1259 */
1260 if (data->sample_freq.hz != 200 && state) {
1261 dev_err(data->dev, "Invalid data rate for tap interrupts.\n");
1262 return -EINVAL;
1263 }
1264
1265 ret = regmap_update_bits(data->regmap, BMA400_INT12_MAP_REG,
1266 BMA400_S_TAP_MSK,
1267 FIELD_PREP(BMA400_S_TAP_MSK, state));
1268 if (ret)
1269 return ret;
1270
1271 switch (dir) {
1272 case IIO_EV_DIR_SINGLETAP:
1273 mask = BMA400_S_TAP_MSK;
1274 set_mask_bits(&field_value, BMA400_S_TAP_MSK,
^^^^^^^^^^^^
Smatch ought to print a warning here but these macros use a bunch of
*(&(*(&field_value))) permutations that confuse Smatch. Smatch does
not print a warning if you're just taking the address of a variable.
This initializes BIT(3)? I believe that KMSan will detect this as a bug
an issue a warning here.
1275 FIELD_PREP(BMA400_S_TAP_MSK, state));
1276 break;
1277 case IIO_EV_DIR_DOUBLETAP:
1278 mask = BMA400_D_TAP_MSK;
1279 set_mask_bits(&field_value, BMA400_D_TAP_MSK,
1280 FIELD_PREP(BMA400_D_TAP_MSK, state));
1281 break;
1282 default:
1283 return -EINVAL;
1284 }
1285
1286 ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG, mask,
--> 1287 field_value);
This uses BIT(3). The function only cares about BIT(3) but the other
bits are uninitialized.
1288 if (ret)
1289 return ret;
1290
1291 set_mask_bits(&data->tap_event_en_bitmask, mask, field_value);
1292
1293 return 0;
1294 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] iio: accel: bma400: Add support for single and double tap events
2022-09-15 14:27 [bug report] iio: accel: bma400: Add support for single and double tap events Dan Carpenter
@ 2022-09-17 13:30 ` Jonathan Cameron
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2022-09-17 13:30 UTC (permalink / raw)
To: Dan Carpenter; +Cc: jagathjog1996, Alexander Potapenko, linux-iio
On Thu, 15 Sep 2022 17:27:43 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello Jagath Jog J,
>
> The patch 961db2da159d: "iio: accel: bma400: Add support for single
> and double tap events" from Aug 31, 2022, leads to the following
> Smatch static checker warning:
>
> drivers/iio/accel/bma400_core.c:1287 bma400_tap_event_en()
> error: uninitialized symbol 'field_value'.
>
> drivers/iio/accel/bma400_core.c
> 1242 static int bma400_tap_event_en(struct bma400_data *data,
> 1243 enum iio_event_direction dir, int state)
> 1244 {
> 1245 unsigned int mask, field_value;
> ^^^^^^^^^^^^
> This is uninitialized.
>
> 1246 int ret;
> 1247
> 1248 /*
> 1249 * Tap interrupts can be configured only in normal mode.
> 1250 * See table in section 4.3 "Power modes - performance modes" of
> 1251 * datasheet v1.2.
> 1252 */
> 1253 if (data->power_mode != POWER_MODE_NORMAL)
> 1254 return -EINVAL;
> 1255
> 1256 /*
> 1257 * Tap interrupts are operating with a data rate of 200Hz.
> 1258 * See section 4.7 "Tap sensing interrupt" in datasheet v1.2.
> 1259 */
> 1260 if (data->sample_freq.hz != 200 && state) {
> 1261 dev_err(data->dev, "Invalid data rate for tap interrupts.\n");
> 1262 return -EINVAL;
> 1263 }
> 1264
> 1265 ret = regmap_update_bits(data->regmap, BMA400_INT12_MAP_REG,
> 1266 BMA400_S_TAP_MSK,
> 1267 FIELD_PREP(BMA400_S_TAP_MSK, state));
> 1268 if (ret)
> 1269 return ret;
> 1270
> 1271 switch (dir) {
> 1272 case IIO_EV_DIR_SINGLETAP:
> 1273 mask = BMA400_S_TAP_MSK;
> 1274 set_mask_bits(&field_value, BMA400_S_TAP_MSK,
> ^^^^^^^^^^^^
> Smatch ought to print a warning here but these macros use a bunch of
> *(&(*(&field_value))) permutations that confuse Smatch. Smatch does
> not print a warning if you're just taking the address of a variable.
>
> This initializes BIT(3)? I believe that KMSan will detect this as a bug
> an issue a warning here.
>
> 1275 FIELD_PREP(BMA400_S_TAP_MSK, state));
> 1276 break;
> 1277 case IIO_EV_DIR_DOUBLETAP:
> 1278 mask = BMA400_D_TAP_MSK;
> 1279 set_mask_bits(&field_value, BMA400_D_TAP_MSK,
> 1280 FIELD_PREP(BMA400_D_TAP_MSK, state));
> 1281 break;
> 1282 default:
> 1283 return -EINVAL;
> 1284 }
> 1285
> 1286 ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG, mask,
> --> 1287 field_value);
>
> This uses BIT(3). The function only cares about BIT(3) but the other
> bits are uninitialized.
>
> 1288 if (ret)
> 1289 return ret;
> 1290
> 1291 set_mask_bits(&data->tap_event_en_bitmask, mask, field_value);
> 1292
> 1293 return 0;
> 1294 }
>
> regards,
> dan carpenter
Agreed. It's not a bug as such because the other bits are never used, but
it certainly hard for checkers to reason about.
Obvious 'fix' is just set the initial value to 0.
I'll send a patch.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-09-17 13:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15 14:27 [bug report] iio: accel: bma400: Add support for single and double tap events Dan Carpenter
2022-09-17 13:30 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.