PengZheng opened a new issue, #723:
URL: https://github.com/apache/celix/issues/723

   For `am=handle`, it is clear that a handle should be untyped pointer, with 
descriptor 'P'.
   For `am=pre`, the Doxygen documentation says that the type should be 
pre-allocated by the function caller. Let me call these types 
"pre-allocatable". The following plain old C struct should qualify as 
pre-allocatable:
   
   ```C
   struct preallocatable {
       int a;
       int b;
   }
   ```
   
   Is text pre-allocatable? I guess not, since the caller can not predict how 
long a string will be.
   
   ```C
   struct not_sure {
       int a;
       int b;
       struct preallocatable* c;
   }
   ```
   
   Is `not_sure` pre-allocatable? On the service provider side, the callee 
could allocate a  struct preallocatable and fill the c field.
   Then `not_sure` can be serialized successfully. However, on the service 
consumer side, it can caused a use-after-free crash.
   In `jsonRpc_handleReply`, we deal with pre-allocatable argument like 
following:
   
   ```C
   if (meta == DYN_FUNCTION_ARGUMENT_META__PRE_ALLOCATED_OUTPUT) {
                   void* tmp = NULL;
                   void** out = (void **) args[i];
                   size_t size = 0;
   
   
                   argType = dynType_typedPointer_getTypedType(argType);
                   status = jsonSerializer_deserializeJson(argType, result, 
&tmp);
                   if (tmp != NULL) {
                       size = dynType_size(argType);
                       memcpy(*out, tmp, size); // address of freed struct 
preallocatable is copied
                   }
                   
   
                   dynType_free(argType, tmp); // preallocatable is freed
               }
   ```
   
   If we do have a precise definition of pre-allocatability, such errors can be 
catched in `dynFunction_parse`:
   
   ```C
           if (strcmp(meta, "handle") == 0) {
               if (dynType_descriptorType(real) != 'P') {
                   celix_err_pushf("Error 'handle' is only allowed for untyped 
pointer not '%c'", dynType_descriptorType(real));
                   return PARSE_ERROR;
               }
               arg->argumentMeta = DYN_FUNCTION_ARGUMENT_META__HANDLE;
           } else if (strcmp(meta, "pre") == 0) {
               if (!dynType_isPreallocatable(real)) {
                   celix_err_pushf("Error 'am=pre' is only allowed for 
preallocatable type");
                   return PARSE_ERROR;
               }
               arg->argumentMeta = 
DYN_FUNCTION_ARGUMENT_META__PRE_ALLOCATED_OUTPUT;
           }
   ```
   
   Similar considerations also apply to other types. For example, for a 
standard argument to work, it should at least be serializable.
   For exmaple, `int` is serializable, `int *` is serailizable, while `int**` 
is not, because we can not tell from a json null whether `int* == NULL` or 
`int** == NULL`.
   
   Due to the lack of precise definitions of these argument types, I am stuck 
in json_rpc.c. Though it is already 100% line covered, it is still very fragile 
IMHO. @pnoltes
   
   _Originally posted by @PengZheng in 
https://github.com/apache/celix/issues/699#issuecomment-1905582200_
               


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to