<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, May 16, 2021 at 3:38 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The CameraMetadata::updateEntry() functions cast the data pointer to a<br>
void pointer, which is then used internally to call<br>
update_camera_metadata_entry(). If the caller passes a pointer to an<br>
incorrect data type, the behaviour is undefined, with possible crashes<br>
if the incorrect data type is smaller than expected by the Android<br>
metadata library.<br>
<br>
To avoid crashes, make all public updateEntry() functions take typed<br>
pointers, and pass the element size to the internal function. The<br>
element size is then checked against the expected size, and an error<br>
message logged if they don't match. This won't catch incorrect data<br>
types that have the same size as the correct type, but will at least<br>
avoid potential crashes.<br>
<br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
---<br>
src/android/camera_metadata.cpp | 11 ++++++++++-<br>
src/android/camera_metadata.h | 12 +++++++++---<br>
2 files changed, 19 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/src/android/camera_metadata.cpp b/src/android/camera_metadata.cpp<br>
index 59366c50cc16..3eb71d7f7d9d 100644<br>
--- a/src/android/camera_metadata.cpp<br>
+++ b/src/android/camera_metadata.cpp<br>
@@ -137,7 +137,8 @@ bool CameraMetadata::addEntry(uint32_t tag, const void *data, size_t count,<br>
return false;<br>
}<br>
<br>
-bool CameraMetadata::updateEntry(uint32_t tag, const void *data, size_t count)<br>
+bool CameraMetadata::updateEntry(uint32_t tag, const void *data, size_t count,<br>
+ size_t elementSize)<br>
{<br>
if (!valid_)<br>
return false;<br>
@@ -152,6 +153,14 @@ bool CameraMetadata::updateEntry(uint32_t tag, const void *data, size_t count)<br>
return false;<br>
}<br>
<br>
+ if (camera_metadata_type_size[entry.type] != elementSize) {<br>
+ const char *name = get_camera_metadata_tag_name(tag);<br>
+ LOG(CameraMetadata, Error)<br>
+ << "Invalid element size for tag "<br>
+ << (name ? name : "<unknown>") << ": not present";<br></blockquote><div><br></div><div>Should the sizes be printed too?</div><div><br></div><div>Regardless of that,</div><div>Reviewed-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org">hiroh@chromium.org</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ return false;<br>
+ }<br>
+<br>
size_t oldSize =<br>
calculate_camera_metadata_entry_data_size(entry.type,<br>
entry.count);<br>
diff --git a/src/android/camera_metadata.h b/src/android/camera_metadata.h<br>
index d7c8d9df689f..f8f2a0d23aa3 100644<br>
--- a/src/android/camera_metadata.h<br>
+++ b/src/android/camera_metadata.h<br>
@@ -55,7 +55,7 @@ public:<br>
template<typename T><br>
bool updateEntry(uint32_t tag, const T &data)<br>
{<br>
- return updateEntry(tag, &data, 1);<br>
+ return updateEntry(tag, &data, 1, sizeof(T));<br>
}<br>
<br>
template<typename T, size_t size><br>
@@ -68,10 +68,14 @@ public:<br>
typename T = typename S::value_type><br>
bool updateEntry(uint32_t tag, const S &data)<br>
{<br>
- return updateEntry(tag, data.data(), data.size());<br>
+ return updateEntry(tag, data.data(), data.size(), sizeof(T));<br>
}<br>
<br>
- bool updateEntry(uint32_t tag, const void *data, size_t count);<br>
+ template<typename T><br>
+ bool updateEntry(uint32_t tag, const T *data, size_t count)<br>
+ {<br>
+ return updateEntry(tag, data, count, sizeof(T));<br>
+ }<br>
<br>
camera_metadata_t *get();<br>
const camera_metadata_t *get() const;<br>
@@ -80,6 +84,8 @@ private:<br>
bool resize(size_t count, size_t size);<br>
bool addEntry(uint32_t tag, const void *data, size_t count,<br>
size_t elementSize);<br>
+ bool updateEntry(uint32_t tag, const void *data, size_t count,<br>
+ size_t elementSize);<br>
<br>
camera_metadata_t *metadata_;<br>
bool valid_;<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>