v1 -> v2: - init_codec to always update with latest payload from firmware (Dmitry/Bryan) - Rewrite the logic of packet parsing to consider payload size for different packet type (Bryan) - Consider reading sfr data till available space (Dmitry) - Add reviewed-by tags
v1: https://lore.kernel.org/all/20241105-venus_oob-v1-0-8d4feedfe2bb@quicinc.com...
This series primarily adds check at relevant places in venus driver where there are possible OOB accesses due to unexpected payload from venus firmware. The patches describes the specific OOB possibility.
Please review and share your feedback.
Validated on sc7180(v4) and rb5(v6).
Stan, please help to extend the test on db410c(v1).
Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com --- Vikash Garodia (4): media: venus: hfi_parser: add check to avoid out of bound access media: venus: hfi_parser: avoid OOB access beyond payload word count media: venus: hfi: add check to handle incorrect queue size media: venus: hfi: add a check to handle OOB in sfr region
drivers/media/platform/qcom/venus/hfi_parser.c | 58 +++++++++++++++++++++----- drivers/media/platform/qcom/venus/hfi_venus.c | 15 ++++++- 2 files changed, 60 insertions(+), 13 deletions(-) --- base-commit: c7ccf3683ac9746b263b0502255f5ce47f64fe0a change-id: 20241115-venus_oob_2-21708239176a
Best regards,
There is a possibility that init_codecs is invoked multiple times during manipulated payload from video firmware. In such case, if codecs_count can get incremented to value more than MAX_CODEC_NUM, there can be OOB access. Reset the count so that it always starts from beginning.
Cc: stable@vger.kernel.org Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser") Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com --- drivers/media/platform/qcom/venus/hfi_parser.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c index 3df241dc3a118bcdeb2c28a6ffdb907b644d5653..1cc17f3dc8948160ea6c3015d2c03e475b8aa29e 100644 --- a/drivers/media/platform/qcom/venus/hfi_parser.c +++ b/drivers/media/platform/qcom/venus/hfi_parser.c @@ -17,6 +17,7 @@ typedef void (*func)(struct hfi_plat_caps *cap, const void *data, static void init_codecs(struct venus_core *core) { struct hfi_plat_caps *caps = core->caps, *cap; + core->codecs_count = 0; unsigned long bit;
if (hweight_long(core->dec_codecs) + hweight_long(core->enc_codecs) > MAX_CODEC_NUM)
On 28/11/2024 05:05, Vikash Garodia wrote:
There is a possibility that init_codecs is invoked multiple times during manipulated payload from video firmware. In such case, if codecs_count can get incremented to value more than MAX_CODEC_NUM, there can be OOB access. Reset the count so that it always starts from beginning.
Cc: stable@vger.kernel.org Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser") Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com
drivers/media/platform/qcom/venus/hfi_parser.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c index 3df241dc3a118bcdeb2c28a6ffdb907b644d5653..1cc17f3dc8948160ea6c3015d2c03e475b8aa29e 100644 --- a/drivers/media/platform/qcom/venus/hfi_parser.c +++ b/drivers/media/platform/qcom/venus/hfi_parser.c @@ -17,6 +17,7 @@ typedef void (*func)(struct hfi_plat_caps *cap, const void *data, static void init_codecs(struct venus_core *core) { struct hfi_plat_caps *caps = core->caps, *cap;
- core->codecs_count = 0; unsigned long bit;
if (hweight_long(core->dec_codecs) + hweight_long(core->enc_codecs) > MAX_CODEC_NUM)
Reviewed-by: Bryan O'Donoghue bryan.odonoghue@linaro.org
words_count denotes the number of words in total payload, while data points to payload of various property within it. When words_count reaches last word, data can access memory beyond the total payload. This can lead to OOB access. Refactor the parsing logic such that the remaining payload is checked before parsing it.
Cc: stable@vger.kernel.org Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser") Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com --- drivers/media/platform/qcom/venus/hfi_parser.c | 57 +++++++++++++++++++++----- 1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c index 1cc17f3dc8948160ea6c3015d2c03e475b8aa29e..14349c2f84b205a8b79dee3acff1408bb63ac54a 100644 --- a/drivers/media/platform/qcom/venus/hfi_parser.c +++ b/drivers/media/platform/qcom/venus/hfi_parser.c @@ -282,8 +282,8 @@ static int hfi_platform_parser(struct venus_core *core, struct venus_inst *inst) u32 hfi_parser(struct venus_core *core, struct venus_inst *inst, void *buf, u32 size) { + u32 *words = buf, *payload, codecs = 0, domain = 0; unsigned int words_count = size >> 2; - u32 *word = buf, *data, codecs = 0, domain = 0; int ret;
ret = hfi_platform_parser(core, inst); @@ -301,36 +301,71 @@ u32 hfi_parser(struct venus_core *core, struct venus_inst *inst, void *buf, }
while (words_count) { - data = word + 1; + payload = words + 1;
- switch (*word) { + switch (*words) { case HFI_PROPERTY_PARAM_CODEC_SUPPORTED: - parse_codecs(core, data); + if (words_count < sizeof(struct hfi_codec_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES; + + parse_codecs(core, payload); init_codecs(core); + words_count -= sizeof(struct hfi_codec_supported); + words += sizeof(struct hfi_codec_supported); break; case HFI_PROPERTY_PARAM_MAX_SESSIONS_SUPPORTED: - parse_max_sessions(core, data); + if (words_count < sizeof(struct hfi_max_sessions_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES; + + parse_max_sessions(core, payload); + words_count -= sizeof(struct hfi_max_sessions_supported); + words += sizeof(struct hfi_max_sessions_supported); break; case HFI_PROPERTY_PARAM_CODEC_MASK_SUPPORTED: - parse_codecs_mask(&codecs, &domain, data); + if (words_count < sizeof(struct hfi_codec_mask_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES; + + parse_codecs_mask(&codecs, &domain, payload); + words_count -= sizeof(struct hfi_codec_mask_supported); + words += sizeof(struct hfi_codec_mask_supported); break; case HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SUPPORTED: - parse_raw_formats(core, codecs, domain, data); + if (words_count < sizeof(struct hfi_uncompressed_format_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES; + + parse_raw_formats(core, codecs, domain, payload); + words_count -= sizeof(struct hfi_uncompressed_format_supported); + words += sizeof(struct hfi_uncompressed_format_supported); break; case HFI_PROPERTY_PARAM_CAPABILITY_SUPPORTED: - parse_caps(core, codecs, domain, data); + if (words_count < sizeof(struct hfi_capabilities)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES; + + parse_caps(core, codecs, domain, payload); + words_count -= sizeof(struct hfi_capabilities); + words += sizeof(struct hfi_capabilities); break; case HFI_PROPERTY_PARAM_PROFILE_LEVEL_SUPPORTED: - parse_profile_level(core, codecs, domain, data); + if (words_count < sizeof(struct hfi_profile_level_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES; + + parse_profile_level(core, codecs, domain, payload); + words_count -= sizeof(struct hfi_profile_level_supported); + words += sizeof(struct hfi_profile_level_supported); break; case HFI_PROPERTY_PARAM_BUFFER_ALLOC_MODE_SUPPORTED: - parse_alloc_mode(core, codecs, domain, data); + if (words_count < sizeof(struct hfi_buffer_alloc_mode_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES; + + parse_alloc_mode(core, codecs, domain, payload); + words_count -= sizeof(struct hfi_buffer_alloc_mode_supported); + words += sizeof(struct hfi_buffer_alloc_mode_supported); break; default: break; }
- word++; + words++; words_count--; }
On 28/11/2024 05:05, Vikash Garodia wrote:
words_count denotes the number of words in total payload, while data points to payload of various property within it. When words_count reaches last word, data can access memory beyond the total payload. This can lead to OOB access. Refactor the parsing logic such that the remaining payload is checked before parsing it.
Cc: stable@vger.kernel.org Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser") Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com
drivers/media/platform/qcom/venus/hfi_parser.c | 57 +++++++++++++++++++++----- 1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c index 1cc17f3dc8948160ea6c3015d2c03e475b8aa29e..14349c2f84b205a8b79dee3acff1408bb63ac54a 100644 --- a/drivers/media/platform/qcom/venus/hfi_parser.c +++ b/drivers/media/platform/qcom/venus/hfi_parser.c @@ -282,8 +282,8 @@ static int hfi_platform_parser(struct venus_core *core, struct venus_inst *inst) u32 hfi_parser(struct venus_core *core, struct venus_inst *inst, void *buf, u32 size) {
- u32 *words = buf, *payload, codecs = 0, domain = 0; unsigned int words_count = size >> 2;
- u32 *word = buf, *data, codecs = 0, domain = 0; int ret;
ret = hfi_platform_parser(core, inst); @@ -301,36 +301,71 @@ u32 hfi_parser(struct venus_core *core, struct venus_inst *inst, void *buf, } while (words_count) {
data = word + 1;
payload = words + 1;
switch (*word) {
case HFI_PROPERTY_PARAM_CODEC_SUPPORTED:switch (*words) {
parse_codecs(core, data);
if (words_count < sizeof(struct hfi_codec_supported))
return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
parse_codecs(core, payload); init_codecs(core);
words_count -= sizeof(struct hfi_codec_supported);
case HFI_PROPERTY_PARAM_MAX_SESSIONS_SUPPORTED:words += sizeof(struct hfi_codec_supported); break;
parse_max_sessions(core, data);
if (words_count < sizeof(struct hfi_max_sessions_supported))
return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
parse_max_sessions(core, payload);
words_count -= sizeof(struct hfi_max_sessions_supported);
case HFI_PROPERTY_PARAM_CODEC_MASK_SUPPORTED:words += sizeof(struct hfi_max_sessions_supported); break;
parse_codecs_mask(&codecs, &domain, data);
if (words_count < sizeof(struct hfi_codec_mask_supported))
return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
parse_codecs_mask(&codecs, &domain, payload);
words_count -= sizeof(struct hfi_codec_mask_supported);
case HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SUPPORTED:words += sizeof(struct hfi_codec_mask_supported); break;
parse_raw_formats(core, codecs, domain, data);
if (words_count < sizeof(struct hfi_uncompressed_format_supported))
return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
parse_raw_formats(core, codecs, domain, payload);
words_count -= sizeof(struct hfi_uncompressed_format_supported);
case HFI_PROPERTY_PARAM_CAPABILITY_SUPPORTED:words += sizeof(struct hfi_uncompressed_format_supported); break;
parse_caps(core, codecs, domain, data);
if (words_count < sizeof(struct hfi_capabilities))
return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
parse_caps(core, codecs, domain, payload);
words_count -= sizeof(struct hfi_capabilities);
case HFI_PROPERTY_PARAM_PROFILE_LEVEL_SUPPORTED:words += sizeof(struct hfi_capabilities); break;
parse_profile_level(core, codecs, domain, data);
if (words_count < sizeof(struct hfi_profile_level_supported))
return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
parse_profile_level(core, codecs, domain, payload);
words_count -= sizeof(struct hfi_profile_level_supported);
case HFI_PROPERTY_PARAM_BUFFER_ALLOC_MODE_SUPPORTED:words += sizeof(struct hfi_profile_level_supported); break;
parse_alloc_mode(core, codecs, domain, data);
if (words_count < sizeof(struct hfi_buffer_alloc_mode_supported))
return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
parse_alloc_mode(core, codecs, domain, payload);
words_count -= sizeof(struct hfi_buffer_alloc_mode_supported);
default: break; }words += sizeof(struct hfi_buffer_alloc_mode_supported); break;
word++;
words_count--; }words++;
I like the changes made here.
Let me suggest you have the parse_something() return the size of the buffer consumed or an error code.
If you calculate the maximum pointer instead of the words_count
frame_size = payload + max;
/* Your while can look like this */
while (words < frame_size) switch(*words){ case HFI_PROPERTY_X: /* if the function returns the bytes consumed */ ret = parse_x(); break; case HFI_PROPERTY_X: ret = parse_x(); break; }
if (ret < 0) return -ret;
/* you can increment the pointer once at the bottom of the loop */ words += ret; }
That way you can
1. Get rid of words_count and not have to decrement it 2. Have one variable words which is checked against the maximum size while(words < frame_size) 3. Have the function that consumes the data return how much buffer it has consumed, instead of inlining in the switch 4. Increment at the bottom of the switch once instead of several times in the switch
IMO it would be clearer/neater that way. Please consider.
--- bod
On 12/2/2024 5:38 PM, Bryan O'Donoghue wrote:
On 28/11/2024 05:05, Vikash Garodia wrote:
words_count denotes the number of words in total payload, while data points to payload of various property within it. When words_count reaches last word, data can access memory beyond the total payload. This can lead to OOB access. Refactor the parsing logic such that the remaining payload is checked before parsing it.
Cc: stable@vger.kernel.org Fixes: 1a73374a04e5 ("media: venus: hfi_parser: add common capability parser") Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com
drivers/media/platform/qcom/venus/hfi_parser.c | 57 +++++++++++++++++++++----- 1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/hfi_parser.c b/drivers/media/platform/qcom/venus/hfi_parser.c index 1cc17f3dc8948160ea6c3015d2c03e475b8aa29e..14349c2f84b205a8b79dee3acff1408bb63ac54a 100644 --- a/drivers/media/platform/qcom/venus/hfi_parser.c +++ b/drivers/media/platform/qcom/venus/hfi_parser.c @@ -282,8 +282,8 @@ static int hfi_platform_parser(struct venus_core *core, struct venus_inst *inst) u32 hfi_parser(struct venus_core *core, struct venus_inst *inst, void *buf, u32 size) { + u32 *words = buf, *payload, codecs = 0, domain = 0; unsigned int words_count = size >> 2; - u32 *word = buf, *data, codecs = 0, domain = 0; int ret; ret = hfi_platform_parser(core, inst); @@ -301,36 +301,71 @@ u32 hfi_parser(struct venus_core *core, struct venus_inst *inst, void *buf, } while (words_count) { - data = word + 1; + payload = words + 1; - switch (*word) { + switch (*words) { case HFI_PROPERTY_PARAM_CODEC_SUPPORTED: - parse_codecs(core, data); + if (words_count < sizeof(struct hfi_codec_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
+ parse_codecs(core, payload); init_codecs(core); + words_count -= sizeof(struct hfi_codec_supported); + words += sizeof(struct hfi_codec_supported); break; case HFI_PROPERTY_PARAM_MAX_SESSIONS_SUPPORTED: - parse_max_sessions(core, data); + if (words_count < sizeof(struct hfi_max_sessions_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
+ parse_max_sessions(core, payload); + words_count -= sizeof(struct hfi_max_sessions_supported); + words += sizeof(struct hfi_max_sessions_supported); break; case HFI_PROPERTY_PARAM_CODEC_MASK_SUPPORTED: - parse_codecs_mask(&codecs, &domain, data); + if (words_count < sizeof(struct hfi_codec_mask_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
+ parse_codecs_mask(&codecs, &domain, payload); + words_count -= sizeof(struct hfi_codec_mask_supported); + words += sizeof(struct hfi_codec_mask_supported); break; case HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SUPPORTED: - parse_raw_formats(core, codecs, domain, data); + if (words_count < sizeof(struct hfi_uncompressed_format_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
+ parse_raw_formats(core, codecs, domain, payload); + words_count -= sizeof(struct hfi_uncompressed_format_supported); + words += sizeof(struct hfi_uncompressed_format_supported); break; case HFI_PROPERTY_PARAM_CAPABILITY_SUPPORTED: - parse_caps(core, codecs, domain, data); + if (words_count < sizeof(struct hfi_capabilities)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
+ parse_caps(core, codecs, domain, payload); + words_count -= sizeof(struct hfi_capabilities); + words += sizeof(struct hfi_capabilities); break; case HFI_PROPERTY_PARAM_PROFILE_LEVEL_SUPPORTED: - parse_profile_level(core, codecs, domain, data); + if (words_count < sizeof(struct hfi_profile_level_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
+ parse_profile_level(core, codecs, domain, payload); + words_count -= sizeof(struct hfi_profile_level_supported); + words += sizeof(struct hfi_profile_level_supported); break; case HFI_PROPERTY_PARAM_BUFFER_ALLOC_MODE_SUPPORTED: - parse_alloc_mode(core, codecs, domain, data); + if (words_count < sizeof(struct hfi_buffer_alloc_mode_supported)) + return HFI_ERR_SYS_INSUFFICIENT_RESOURCES;
+ parse_alloc_mode(core, codecs, domain, payload); + words_count -= sizeof(struct hfi_buffer_alloc_mode_supported); + words += sizeof(struct hfi_buffer_alloc_mode_supported); break; default: break; } - word++; + words++; words_count--; }
I like the changes made here.
Let me suggest you have the parse_something() return the size of the buffer consumed or an error code.
If you calculate the maximum pointer instead of the words_count
frame_size = payload + max;
/* Your while can look like this */
while (words < frame_size) switch(*words){ case HFI_PROPERTY_X: /* if the function returns the bytes consumed */ ret = parse_x(); break; case HFI_PROPERTY_X: ret = parse_x(); break; }
if (ret < 0) return -ret;
/* you can increment the pointer once at the bottom of the loop */ words += ret; }
That way you can
- Get rid of words_count and not have to decrement it
- Have one variable words which is checked against the maximum
size while(words < frame_size) 3. Have the function that consumes the data return how much buffer it has consumed, instead of inlining in the switch 4. Increment at the bottom of the switch once instead of several times in the switch
IMO it would be clearer/neater that way. Please consider.
Appreciate your time to dig deeper into it. Expanding your suggestion, filling in the details
frame_size = words + size;
/* Your while can look like this */
while (words < frame_size) remaining_size = framesize - words; switch(*words){ case HFI_PROPERTY_X: if (remaining_size < sizeof(payload_X) return insuff_res; /* if the function returns the bytes consumed */ ret = parse_x(core, words + 1); break; case HFI_PROPERTY_Y: if (remaining_size < sizeof(payload_X) return insuff_res; ret = parse_y(core, words + 1); break; default: ret = 1; }
if (ret < 0) return -ret;
/* you can increment the pointer once at the bottom of the loop */ words += ret; }
If you see, words_count is doing the role of remaining_size. In existing implementation as well, we can move those increments per case to once per loop, just that to avoid incrementing for default case.
Regards, Vikash
On 02/12/2024 13:24, Vikash Garodia wrote:
If you see, words_count is doing the role of remaining_size. In existing implementation as well, we can move those increments per case to once per loop, just that to avoid incrementing for default case.
Yes.
To me it seems
- A redundant step to have words_count - That the functions themselves should return the amount of bytes words += should increment by - That you could do that words += outside of the switch instead of at each case:
but to be clear the logic of incrementing the words looks right to me now, I'm suggesting additional stylistic change.
--- bod
qsize represents size of shared queued between driver and video firmware. Firmware can modify this value to an invalid large value. In such situation, empty_space will be bigger than the space actually available. Since new_wr_idx is not checked, so the following code will result in an OOB write. ... qsize = qhdr->q_size
if (wr_idx >= rd_idx) empty_space = qsize - (wr_idx - rd_idx) .... if (new_wr_idx < qsize) { memcpy(wr_ptr, packet, dwords << 2) --> OOB write
Add check to ensure qsize is within the allocated size while reading and writing packets into the queue.
Cc: stable@vger.kernel.org Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files") Reviewed-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com --- drivers/media/platform/qcom/venus/hfi_venus.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c index f9437b6412b91c2483670a2b11f4fd43f3206404..6b615270c5dae470c6fad408c9b5bc037883e56e 100644 --- a/drivers/media/platform/qcom/venus/hfi_venus.c +++ b/drivers/media/platform/qcom/venus/hfi_venus.c @@ -187,6 +187,9 @@ static int venus_write_queue(struct venus_hfi_device *hdev, /* ensure rd/wr indices's are read from memory */ rmb();
+ if (qsize > IFACEQ_QUEUE_SIZE / 4) + return -EINVAL; + if (wr_idx >= rd_idx) empty_space = qsize - (wr_idx - rd_idx); else @@ -255,6 +258,9 @@ static int venus_read_queue(struct venus_hfi_device *hdev, wr_idx = qhdr->write_idx; qsize = qhdr->q_size;
+ if (qsize > IFACEQ_QUEUE_SIZE / 4) + return -EINVAL; + /* make sure data is valid before using it */ rmb();
sfr->buf_size is in shared memory and can be modified by malicious user. OOB write is possible when the size is made higher than actual sfr data buffer. Cap the size to allocated size for such cases.
Cc: stable@vger.kernel.org Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files") Reviewed-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Signed-off-by: Vikash Garodia quic_vgarodia@quicinc.com --- drivers/media/platform/qcom/venus/hfi_venus.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c index 6b615270c5dae470c6fad408c9b5bc037883e56e..c3113420d266e61fcab44688580288d7408b50f4 100644 --- a/drivers/media/platform/qcom/venus/hfi_venus.c +++ b/drivers/media/platform/qcom/venus/hfi_venus.c @@ -1041,18 +1041,23 @@ static void venus_sfr_print(struct venus_hfi_device *hdev) { struct device *dev = hdev->core->dev; struct hfi_sfr *sfr = hdev->sfr.kva; + u32 size; void *p;
if (!sfr) return;
- p = memchr(sfr->data, '\0', sfr->buf_size); + size = sfr->buf_size; + if (size > ALIGNED_SFR_SIZE) + size = ALIGNED_SFR_SIZE; + + p = memchr(sfr->data, '\0', size); /* * SFR isn't guaranteed to be NULL terminated since SYS_ERROR indicates * that Venus is in the process of crashing. */ if (!p) - sfr->data[sfr->buf_size - 1] = '\0'; + sfr->data[size - 1] = '\0';
dev_err_ratelimited(dev, "SFR message from FW: %s\n", sfr->data); }
linux-stable-mirror@lists.linaro.org