soc: apple: rtkit: harden syslog shared-buffer handling
Subsystems (2)
v2 cover soc: apple: rtkit: harden syslog shared-buffer handling
Split the syslog hardening into focused changes as requested. The first patch fixes the SYSLOG_INIT reinitialization leak. The second prevents zero-sized messages and out-of-bounds shared-memory copies. The third corrects the entry-count boundary, and the fourth rate limits validation warnings controlled by the coprocessor.
The index-boundary change has not been tested on Apple hardware running Linux. Asahi m1n1 describes the initialization field as COUNT, consistent with the kernel's syslog_n_entries name, but patch 3 is kept independent so it can be reverted without removing the shared-memory bounds protection.
Testing:
- arm64 GCC build of drivers/soc/apple/rtkit.o with W=1 in Docker
- scripts/checkpatch.pl --strict on each patch
Changes since v1:
- inline the one-use shared-memory bounds check
- use == 0 for the crashlog copy and < 0 for syslog copies
- split the SYSLOG_INIT leak fix into patch 1
- split the idx == n_entries change into patch 3
- make the zero-size guard independent of the leak fix
- rate limit existing malformed-syslog warnings in patch 4
- rebase onto upstream commit 0a0d1d55dad5
Laxman Acharya Padhya (4):
soc: apple: rtkit: free syslog buffer before reinitializing
soc: apple: rtkit: bound syslog copies to the shared buffer
soc: apple: rtkit: reject syslog indices outside the entry count
soc: apple: rtkit: rate limit malformed syslog warnings
drivers/soc/apple/rtkit.c | 75 +++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 26 deletions(-)
base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b
Signature · 1 line
2.51.2
PATCH v2 1/4 soc: apple: rtkit: free syslog buffer before reinitializing
A second SYSLOG_INIT message overwrites syslog_msg_buffer without freeing the previous allocation. Free the old buffer before allocating its replacement.
--- drivers/soc/apple/rtkit.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/soc/apple/rtkit.c +3 -0
@@ -422,6 +422,9 @@ static void apple_rtkit_ioreport_rx(struct apple_rtkit *rtk, u64 msg)static void apple_rtkit_syslog_rx_init (struct apple_rtkit *rtk,u64 msg) { +kfree (rtk->syslog_msg_buffer); + rtk->syslog_msg_buffer =NULL ; + rtk->syslog_n_entries =FIELD_GET (APPLE_RTKIT_SYSLOG_N_ENTRIES , msg); rtk->syslog_msg_size =FIELD_GET (APPLE_RTKIT_SYSLOG_MSG_SIZE , msg);
Signature · 1 line
2.51.2
PATCH v2 2/4 soc: apple: rtkit: bound syslog copies to the shared buffer
In reply to Laxman Acharya Padhyaapple_rtkit_syslog_rx_log() copies a log entry out of the coprocessor shared-memory ring using an index and layout chosen by the coprocessor. These values are not checked against syslog_buffer.size, so buggy firmware can make memcpy_fromio() or memcpy() read past the DMA mapping.
SYSLOG_INIT also accepts a zero message size. kzalloc(0) returns ZERO_SIZE_PTR, which is non-NULL, and the later strnlen() size wraps from zero to SIZE_MAX.
Reject zero-sized messages and make apple_rtkit_memcpy() refuse copies that do not fit in the shared buffer. Rate limit the bounds warning so the coprocessor cannot flood the kernel log.
--- drivers/soc/apple/rtkit.c | 49 ++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 14 deletions(-)
drivers/soc/apple/rtkit.c +35 -14
@@ -348,14 +348,23 @@ static void apple_rtkit_free_buffer(struct apple_rtkit *rtk, bfr->is_mapped = false; } -static void apple_rtkit_memcpy (struct apple_rtkit *rtk,void *dst, -struct apple_rtkit_shmem *bfr,size_t offset, -size_t len) -{ +static int apple_rtkit_memcpy (struct apple_rtkit *rtk,void *dst, +struct apple_rtkit_shmem *bfr,size_t offset, +size_t len) +{ +if (offset > bfr->size || len > bfr->size - offset) { +dev_warn_ratelimited (rtk->dev, +"RTKit: shared-memory copy out of bounds (off 0x%zx len 0x%zx size 0x%zx)\n" , + offset, len, bfr->size); +return -EINVAL ; + } +if (bfr->iomem)memcpy_fromio (dst, bfr->iomem + offset, len);else memcpy (dst, bfr->buffer + offset, len); + +return 0 ; }static void apple_rtkit_crashlog_rx (struct apple_rtkit *rtk,u64 msg)
@@ -384,9 +393,10 @@ static void apple_rtkit_crashlog_rx(struct apple_rtkit *rtk, u64 msg)*/ bfr =kzalloc (rtk->crashlog_buffer.size,GFP_KERNEL );if (bfr) { -apple_rtkit_memcpy (rtk, bfr, &rtk->crashlog_buffer,0 , - rtk->crashlog_buffer.size); -apple_rtkit_crashlog_dump (rtk, bfr, rtk->crashlog_buffer.size); +if (apple_rtkit_memcpy (rtk, bfr, &rtk->crashlog_buffer,0 , + rtk->crashlog_buffer.size) ==0 ) +apple_rtkit_crashlog_dump (rtk, bfr, + rtk->crashlog_buffer.size); }else {dev_err (rtk->dev,"RTKit: Couldn't allocate crashlog shadow buffer\n" );
@@ -428,6 +438,11 @@ static void apple_rtkit_syslog_rx_init(struct apple_rtkit *rtk, u64 msg) rtk->syslog_n_entries =FIELD_GET (APPLE_RTKIT_SYSLOG_N_ENTRIES , msg); rtk->syslog_msg_size =FIELD_GET (APPLE_RTKIT_SYSLOG_MSG_SIZE , msg); +if (rtk->syslog_msg_size ==0 ) { +dev_warn (rtk->dev,"RTKit: syslog msg_size is zero\n" ); +return ; + } + rtk->syslog_msg_buffer =kzalloc (rtk->syslog_msg_size,GFP_KERNEL ); dev_dbg(rtk->dev,
@@ -444,10 +459,11 @@ static void apple_rtkit_syslog_rx_log(struct apple_rtkit *rtk, u64 msg) {u8 idx = msg &0xff ;char log_context[24 ]; -size_t entry_size =0x20 + rtk->syslog_msg_size; +size_t entry_size; +size_t offset;int msglen; -if (!rtk->syslog_msg_buffer) { +if (!rtk->syslog_msg_buffer || rtk->syslog_msg_size ==0 ) {dev_warn ( rtk->dev,"RTKit: received syslog message but no syslog_msg_buffer\n" );
@@ -471,11 +487,16 @@ static void apple_rtkit_syslog_rx_log(struct apple_rtkit *rtk, u64 msg) goto done; } -apple_rtkit_memcpy (rtk, log_context, &rtk->syslog_buffer, - idx * entry_size +8 ,sizeof (log_context)); -apple_rtkit_memcpy (rtk, rtk->syslog_msg_buffer, &rtk->syslog_buffer, - idx * entry_size +8 +sizeof (log_context), - rtk->syslog_msg_size); + entry_size =0x20 + rtk->syslog_msg_size; + offset = (size_t )idx * entry_size +8 ; +if (apple_rtkit_memcpy (rtk, log_context, &rtk->syslog_buffer, offset, +sizeof (log_context)) <0 ) + goto done; +if (apple_rtkit_memcpy (rtk, rtk->syslog_msg_buffer, + &rtk->syslog_buffer, + offset +sizeof (log_context), + rtk->syslog_msg_size) <0 ) + goto done; log_context[sizeof (log_context) -1 ] =0 ;
Signature · 1 line
2.51.2
PATCH v2 3/4 soc: apple: rtkit: reject syslog indices outside the entry count
In reply to Laxman Acharya Padhyasyslog_n_entries is a count, so valid indices range from zero through syslog_n_entries - 1. The current check also accepts syslog_n_entries itself.
Reject that index before deriving the shared-buffer offset. The shared copy bounds check remains the final protection against inconsistent firmware-provided layouts.
--- drivers/soc/apple/rtkit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/soc/apple/rtkit.c +1 -1
@@ -481,7 +481,7 @@ static void apple_rtkit_syslog_rx_log(struct apple_rtkit *rtk, u64 msg) "RTKit: received syslog message but no syslog_buffer.buffer or syslog_buffer.iomem\n"); goto done; } -if (idx > rtk->syslog_n_entries) { +if (idx >= rtk->syslog_n_entries) {dev_warn (rtk->dev,"RTKit: syslog index %d out of range\n" , idx); goto done;
Signature · 1 line
2.51.2
PATCH v2 4/4 soc: apple: rtkit: rate limit malformed syslog warnings
In reply to Laxman Acharya PadhyaThe coprocessor can repeatedly send malformed syslog messages and flood the kernel log with warnings. Use the rate-limited warning helper for the validation failures in the syslog receive path.
--- drivers/soc/apple/rtkit.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
drivers/soc/apple/rtkit.c +11 -12
@@ -439,7 +439,8 @@ static void apple_rtkit_syslog_rx_init(struct apple_rtkit *rtk, u64 msg) rtk->syslog_msg_size =FIELD_GET (APPLE_RTKIT_SYSLOG_MSG_SIZE , msg);if (rtk->syslog_msg_size ==0 ) { -dev_warn (rtk->dev,"RTKit: syslog msg_size is zero\n" ); +dev_warn_ratelimited (rtk->dev, +"RTKit: syslog msg_size is zero\n" );return ; }
@@ -464,26 +465,24 @@ static void apple_rtkit_syslog_rx_log(struct apple_rtkit *rtk, u64 msg)int msglen;if (!rtk->syslog_msg_buffer || rtk->syslog_msg_size ==0 ) { -dev_warn ( - rtk->dev, -"RTKit: received syslog message but no syslog_msg_buffer\n" ); +dev_warn_ratelimited (rtk->dev, +"RTKit: received syslog message but no syslog_msg_buffer\n" ); goto done; }if (!rtk->syslog_buffer.size) { -dev_warn ( - rtk->dev, -"RTKit: received syslog message but syslog_buffer.size is zero\n" ); +dev_warn_ratelimited (rtk->dev, +"RTKit: received syslog message but syslog_buffer.size is zero\n" ); goto done; }if (!rtk->syslog_buffer.buffer && !rtk->syslog_buffer.iomem) { -dev_warn ( - rtk->dev, -"RTKit: received syslog message but no syslog_buffer.buffer or syslog_buffer.iomem\n" ); +dev_warn_ratelimited (rtk->dev, +"RTKit: received syslog message but no syslog_buffer.buffer or syslog_buffer.iomem\n" ); goto done; }if (idx >= rtk->syslog_n_entries) { -dev_warn (rtk->dev,"RTKit: syslog index %d out of range\n" , - idx); +dev_warn_ratelimited (rtk->dev, +"RTKit: syslog index %d out of range\n" , + idx); goto done; }
Signature · 1 line
2.51.2
Sven Peter
maintainerQuoted 6 lines · Laxman Acharya Padhya
Split the syslog hardening into focused changes as requested. The first patch fixes the
SYSLOG_INITreinitialization leak. The second prevents zero-sized messages and out-of-bounds shared-memory copies. The third corrects the entry-count boundary, and the fourth rate limits validation warnings controlled by the coprocessor.
Quoted 8 lines
The index-boundary change has not been tested on Apple hardware running Linux. Asahi m1n1 describes the initialization field as COUNT, consistent with the kernel's
syslog_n_entriesname, but patch 3 is kept independent so it can be reverted without removing the shared-memory bounds protection.Testing:
- arm64 GCC build ofdrivers/soc/apple/rtkit.owith W=1 in Docker
-scripts/checkpatch.pl--strict on each patch
How was this found? If it was AI assisted please also add the appropriate Assisted-by: tags. I'd also like a Tested-by from someone before I pick this up then to make sure you didn't break anything.
Sven