Re: [PATCH for-4.17 v3 14/15] tools/ocaml/xenstored/syslog_stubs.c: avoid potential NULL dereference

[Edwin Torok <edvin.torok@xxxxxxxxxx>]

> On 8 Nov 2022, at 16:07, Julien Grall <julien@xxxxxxx> wrote:
> 
> 
> 
> On 08/11/2022 15:34, Edwin Török wrote:
>> If we are out of memory then strdup may return NULL, and passing NULL to
>> syslog may cause a crash.
>> Avoid this by using `caml_stat_strdup` which will raise an OCaml out of
>> memory exception instead.
>> This then needs to be paired with caml_stat_free.
>> Signed-off-by: Edwin Török <edvin.torok@xxxxxxxxxx>
>> ---
>> Reason for inclusion in 4.17:
>> - fixes a bug in out of memory situations
>> Changes since v2:
>> - new in v3
>> ---
>>  tools/ocaml/xenstored/syslog_stubs.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>> diff --git a/tools/ocaml/xenstored/syslog_stubs.c 
>> b/tools/ocaml/xenstored/syslog_stubs.c
>> index 4e5e49b557..4ad85c8eb5 100644
>> --- a/tools/ocaml/xenstored/syslog_stubs.c
>> +++ b/tools/ocaml/xenstored/syslog_stubs.c
>> @@ -14,6 +14,7 @@
>>    #include <syslog.h>
>>  #include <string.h>
>> +#include <caml/fail.h>
>>  #include <caml/mlvalues.h>
>>  #include <caml/memory.h>
>>  #include <caml/alloc.h>
>> @@ -35,14 +36,16 @@ static int __syslog_facility_table[] = {
>>  value stub_syslog(value facility, value level, value msg)
>>  {
>>      CAMLparam3(facility, level, msg);
>> -    const char *c_msg = strdup(String_val(msg));
>> +    char *c_msg = strdup(String_val(msg));
> 
> This change seems to be unrelated with the goal of the commit. IMHO, this 
> should be done in a separate patch.
> 
> The minimum would be to mention in the commit message.

That is to avoid freeing 'const char*' (there is a typecast below).
I'll mention it.

> 
> Cheers,
> 
> -- 
> Julien Grall