Hi Quang,

One comment inline.

Best Regards,
ThuanTr

-----Original Message-----
From: Quang Xuan Nhat Nghiem <quang.xn.ngh...@dektech.com.au> 
Sent: Wednesday, November 11, 2020 12:56 PM
To: Minh Hon Chau <minh.c...@dektech.com.au>; Thuan Tran 
<thuan.t...@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net; Quang Xuan Nhat Nghiem 
<quang.xn.ngh...@dektech.com.au>
Subject: [PATCH 1/1] ntf: fix coredump while creating object having string 
value, SA_NOTIFY [#3232]

When create or modify an object having size of attribute value over 65535,
this actual size will be truncated because dataSize of saNtfPtrValAllocate
is SaUint16T (from 0 to 65535). Thus, after saNtfPtrValAllocate's invoked,
the attribute value is assigned to the memory allocated with the actual
size over 65535 and cause a memory corruption.
Solution is prevent the size of data and log a warning if is's over 65535.
---
 src/ntf/ntfimcnd/ntfimcn_notifier.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/ntf/ntfimcnd/ntfimcn_notifier.c 
b/src/ntf/ntfimcnd/ntfimcn_notifier.c
index c63b4393f..148e5abae 100644
--- a/src/ntf/ntfimcnd/ntfimcn_notifier.c
+++ b/src/ntf/ntfimcnd/ntfimcn_notifier.c
@@ -233,8 +233,16 @@ static int fill_value_array(SaNtfNotificationHandleT 
notificationHandle,
 
        TRACE_ENTER();
 
-       rc = saNtfPtrValAllocate(notificationHandle, value_in_size,
-                                (void **)&dest_ptr, value_out);
+       if (value_in_size > USHRT_MAX) {
+               LOG_WA("Failed to prepare notification as attr value size "
+                      "(%llu) > MAX(%u)",
+                      value_in_size, USHRT_MAX);
+               internal_rc = (-1);
+               goto done;
+       } else {
+               rc = saNtfPtrValAllocate(notificationHandle, value_in_size,
+                                        (void **)&dest_ptr, value_out);
[Thuan] No need put it in ELSE as IF block has goto done.
               Keep this out of ELSE make smaller changes and consistent code.
               Otherwise, below IF block inside this ELSE looks more consistent.
+       }
        if (rc != SA_AIS_OK) {
                LOG_ER("%s: saNtfPtrValAllocate failed %s", __FUNCTION__,
                       saf_error(rc));
-- 
2.17.1



_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to