| Message ID | 5da5b63bd6a0f8d0f6ad0281773eefb32de8164d.1667920496.git.edvin.torok@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | OCaml fixes for Xen 4.17 | expand |
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@citrix.com> > --- > 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. Cheers,
> On 8 Nov 2022, at 16:07, Julien Grall <julien@xen.org> 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@citrix.com> >> --- >> 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
On 8 Nov 2022, at 15:34, Edwin Török <edvin.torok@citrix.com<mailto:edvin.torok@citrix.com>> 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@citrix.com<mailto:edvin.torok@citrix.com>> --- 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(-) Acked-by: Christian Lindig <christian.lindig@citrix.com<mailto:christian.lindig@citrix.com>>
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)); int c_facility = __syslog_facility_table[Int_val(facility)] | __syslog_level_table[Int_val(level)]; + if ( !c_msg ) + caml_raise_out_of_memory(); caml_enter_blocking_section(); syslog(c_facility, "%s", c_msg); caml_leave_blocking_section(); - free((void*)c_msg); + free(c_msg); CAMLreturn(Val_unit); }
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@citrix.com> --- 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(-)