Our static checker found a bug where set_serial_info() uses a mutex, but
get_serial_info() does not. Fortunately, the impact of this is relatively
minor. It doesn't cause a crash or any other serious issues. However, if a
race condition occurs between set_serial_info() and get_serial_info(),
there is a chance that the data returned by get_serial_info() will be
meaningless.
Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com>
Fixes: 0aad5ad563c8 ("greybus/uart: switch to ->[sg]et_serial()")
---
V2:
Modified the patch description to make it more concise and easier to understand.
Changed the fix code to ensure the logic is correct.
Thanks to Johan Hovold and Dan Carpenter for helpful suggestion.
V3:
Used the correct patch prefix.
Thanks Johan Hovold, Dan Carpenter, Alex Elder and Greg KH for helpful suggestion.
---
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..8eab94cb06fa 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -596,11 +596,13 @@ static int get_serial_info(struct tty_struct *tty,
struct gb_tty *gb_tty = tty->driver_data;
ss->line = gb_tty->minor;
+ mutex_lock(&gb_tty->port.mutex);
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
Our static checker found a bug where set_serial_info() uses a mutex, but
get_serial_info() does not. Fortunately, the impact of this is relatively
minor. It doesn't cause a crash or any other serious issues. However, if a
race condition occurs between set_serial_info() and get_serial_info(),
there is a chance that the data returned by get_serial_info() will be
meaningless.
Signed-off-by: Qiu-ji Chen <chenqiuji666(a)gmail.com>
Fixes: 0aad5ad563c8 ("greybus/uart: switch to ->[sg]et_serial()")
---
V2:
Modified the patch description to make it more concise and easier to understand.
Changed the fix code to ensure the logic is correct.
Thanks to Johan Hovold and Dan Carpenter for helpful suggestion.
---
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..8eab94cb06fa 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -596,11 +596,13 @@ static int get_serial_info(struct tty_struct *tty,
struct gb_tty *gb_tty = tty->driver_data;
ss->line = gb_tty->minor;
+ mutex_lock(&gb_tty->port.mutex);
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
s/interfce/interface/
A 'a' is missing. Add it.
Signed-off-by: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr>
---
drivers/greybus/interface.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c
index d022bfb5e95d..a0f3e9422721 100644
--- a/drivers/greybus/interface.c
+++ b/drivers/greybus/interface.c
@@ -780,7 +780,7 @@ const struct device_type greybus_interface_type = {
* The position of interface within the Endo is encoded in "interface_id"
* argument.
*
- * Returns a pointer to the new interfce or a null pointer if a
+ * Returns a pointer to the new interface or a null pointer if a
* failure occurs due to memory exhaustion.
*/
struct gb_interface *gb_interface_create(struct gb_module *module,
--
2.47.0
Fix an issue detected by the Smatch static tool:
drivers/greybus/operation.c:852 gb_operation_response_send() error:
we previously assumed 'operation->response' could be null (see line 829)
The issue occurs because 'operation->response' may be null if the
response allocation fails at line 829. However, the code tries to
access 'operation->response->header' at line 852 without checking if
it was successfully allocated. This can cause a crash if 'response'
is null.
To fix this, add a check to ensure 'operation->response' is not null
before accessing its header. If the response is null, log an error
message and return -ENOMEM to stop further processing, preventing
any crashes or undefined behavior.
Signed-off-by: Suraj Sonawane <surajsonawane0215(a)gmail.com>
---
drivers/greybus/operation.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/greybus/operation.c b/drivers/greybus/operation.c
index 8459e9bc0..521899fbc 100644
--- a/drivers/greybus/operation.c
+++ b/drivers/greybus/operation.c
@@ -849,7 +849,13 @@ static int gb_operation_response_send(struct gb_operation *operation,
goto err_put;
/* Fill in the response header and send it */
- operation->response->header->result = gb_operation_errno_map(errno);
+ if (operation->response) {
+ operation->response->header->result = gb_operation_errno_map(errno);
+ } else {
+ dev_err(&connection->hd->dev, "failed to allocate response\n");
+ ret = -ENOMEM;
+ goto err_put_active;
+ }
ret = gb_message_send(operation->response, GFP_KERNEL);
if (ret)
--
2.34.1
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>