Atomicity violation occurs during consecutive reads of the members of
gb_tty. Consider a scenario where, because the consecutive reads of gb_tty
members are not protected by a lock, the value of gb_tty may still be
changing during the read process.
gb_tty->port.close_delay and gb_tty->port.closing_wait are updated
together, such as in the set_serial_info() function. If during the
read process, gb_tty->port.close_delay and gb_tty->port.closing_wait
are still being updated, it is possible that gb_tty->port.close_delay
is updated while gb_tty->port.closing_wait is not. In this case,
the code first reads gb_tty->port.close_delay and then
gb_tty->port.closing_wait. A new gb_tty->port.close_delay and an
old gb_tty->port.closing_wait could be read. Such values, whether
before or after the update, should not coexist as they represent an
intermediate state.
This could result in a mismatch of the values read for gb_tty->minor,
gb_tty->port.close_delay, and gb_tty->port.closing_wait, which in turn
could cause ss->close_delay and ss->closing_wait to be mismatched.
To address this issue, we have enclosed all sequential read operations of
the gb_tty variable within a lock. This ensures that the value of gb_tty
remains unchanged throughout the process, guaranteeing its validity.
This possible bug is found by an experimental static analysis tool
developed by our team. This tool analyzes the locking APIs
to extract function pairs that can be concurrently executed, and then
analyzes the instructions in the paired functions to identify possible
concurrency bugs including data races and atomicity violations.
Fixes: b71e571adaa5 ("staging: greybus: uart: fix TIOCSSERIAL jiffies conversions")
Cc: stable(a)vger.kernel.org
Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com>
---
drivers/staging/greybus/uart.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
index cdf4ebb93b10..8cc18590d97b 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -595,12 +595,14 @@ static int get_serial_info(struct tty_struct *tty,
{
struct gb_tty *gb_tty = tty->driver_data;
+ mutex_lock(&gb_tty->port.mutex);
ss->line = gb_tty->minor;
ss->close_delay = jiffies_to_msecs(gb_tty->port.close_delay) / 10;
ss->closing_wait =
gb_tty->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
ASYNC_CLOSING_WAIT_NONE :
jiffies_to_msecs(gb_tty->port.closing_wait) / 10;
+ mutex_unlock(&gb_tty->port.mutex);
return 0;
}
--
2.34.1
Adds support for beagleplay cc1352 co-processor firmware upgrade using
kernel Firmware Upload API. Uses ROM based bootloader present in
cc13x2x7 and cc26x2x7 platforms for flashing over UART.
Communication with the bootloader can be moved out of gb-beagleplay
driver if required, but I am keeping it here since there are no
immediate plans to use the on-board cc1352p7 for anything other than
greybus (BeagleConnect Technology). Additionally, there do not seem to
any other devices using cc1352p7 or its cousins as a co-processor.
Bootloader backdoor and reset GPIOs are used to enable cc1352p7 bootloader
backdoor for flashing. Flashing is skipped in case we are trying to flash
the same image as the one that is currently present. This is determined by
CRC32 calculation of the supplied firmware and flash data.
We also do a CRC32 check after flashing to ensure that the firmware was
flashed properly.
Link: https://www.ti.com/lit/ug/swcu192/swcu192.pdf Ti CC1352P7 Technical Specification
Changes in v4:
- Add acks properly
- Fix Kconfig warning by adding select FW_LOADER
- Link to v3: https://lore.kernel.org/r/20240825-beagleplay_fw_upgrade-v3-0-8f424a9de9f6@…
Changes in v3:
- Spelling fixes in cover letter
- Add Ack by Rob Herring on Patch 1
- Link to v2: https://lore.kernel.org/r/20240801-beagleplay_fw_upgrade-v2-0-e36928b792db@…
Changes in v2:
- Spelling fixes
- Rename boot-gpios to bootloader-backdoor-gpios
- Add doc comments
- Add check to ensure firmware size is 704 KB
- Link to v1: https://lore.kernel.org/all/20240719-beagleplay_fw_upgrade-v1-0-8664d451325…
Signed-off-by: Ayush Singh <ayush(a)beagleboard.org>
---
Ayush Singh (3):
dt-bindings: net: ti,cc1352p7: Add bootloader-backdoor-gpios
arm64: dts: ti: k3-am625-beagleplay: Add bootloader-backdoor-gpios to cc1352p7
greybus: gb-beagleplay: Add firmware upload API
.../devicetree/bindings/net/ti,cc1352p7.yaml | 7 +
arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 3 +-
drivers/greybus/Kconfig | 2 +
drivers/greybus/gb-beagleplay.c | 658 ++++++++++++++++++++-
4 files changed, 656 insertions(+), 14 deletions(-)
---
base-commit: f76698bd9a8ca01d3581236082d786e9a6b72bb7
change-id: 20240715-beagleplay_fw_upgrade-43e6cceb0d3d
Best regards,
--
Ayush Singh <ayush(a)beagleboard.org>
Since the debugfs_create_dir() never returns a null pointer, checking
the return value for a null pointer is redundant, and using IS_ERR is
safe enough.
Signed-off-by: Li Zetao <lizetao1(a)huawei.com>
---
drivers/greybus/svc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/greybus/svc.c b/drivers/greybus/svc.c
index 4256467fcd35..906ea61cba30 100644
--- a/drivers/greybus/svc.c
+++ b/drivers/greybus/svc.c
@@ -765,7 +765,7 @@ static void gb_svc_pwrmon_debugfs_init(struct gb_svc *svc)
u8 rail_count;
dent = debugfs_create_dir("pwrmon", svc->debugfs_dentry);
- if (IS_ERR_OR_NULL(dent))
+ if (IS_ERR(dent))
return;
if (gb_svc_pwrmon_rail_count_get(svc, &rail_count))
--
2.34.1