| Message ID | 20241119213142.77248-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: fix allocation for s390x loadparm | expand |
On 11/19/24 4:31 PM, Paolo Bonzini wrote: > Coverity reports a possible buffer overrun due to a non-NUL-terminated > string in scsi_property_set_loadparm(). While things are not so easy, > because qdev_prop_sanitize_s390x_loadparm is designed to operate on a > buffer that is not NUL-terminated, in this case the string *does* have > to be NUL-terminated because it is read by scsi_property_get_loadparm > and s390_build_iplb. > > Cc: jrossi@linux.ibm.com > Cc: thuth@redhat.com > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/scsi/scsi-disk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > index 8e553487d50..7f13b0588f2 100644 > --- a/hw/scsi/scsi-disk.c > +++ b/hw/scsi/scsi-disk.c > @@ -3152,7 +3152,7 @@ static void scsi_property_set_loadparm(Object *obj, const char *value, > return; > } > > - lp_str = g_malloc0(strlen(value)); > + lp_str = g_malloc0(strlen(value) + 1); > if (!qdev_prop_sanitize_s390x_loadparm(lp_str, value, errp)) { > g_free(lp_str); > return; Reviewed-by Jared Rossi <jrossi@linux.ibm.com>
On 19/11/2024 22.31, Paolo Bonzini wrote: > Coverity reports a possible buffer overrun due to a non-NUL-terminated > string in scsi_property_set_loadparm(). While things are not so easy, > because qdev_prop_sanitize_s390x_loadparm is designed to operate on a > buffer that is not NUL-terminated, in this case the string *does* have > to be NUL-terminated because it is read by scsi_property_get_loadparm > and s390_build_iplb. > > Cc: jrossi@linux.ibm.com > Cc: thuth@redhat.com > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Fixes: 429442e52d ("hw: Add "loadparm" property to scsi disk devices for booting on s390x") > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c > index 8e553487d50..7f13b0588f2 100644 > --- a/hw/scsi/scsi-disk.c > +++ b/hw/scsi/scsi-disk.c > @@ -3152,7 +3152,7 @@ static void scsi_property_set_loadparm(Object *obj, const char *value, > return; > } > > - lp_str = g_malloc0(strlen(value)); > + lp_str = g_malloc0(strlen(value) + 1); D'oh, me looks for a brown paperbag for hiding... sorry for that one. Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 8e553487d50..7f13b0588f2 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -3152,7 +3152,7 @@ static void scsi_property_set_loadparm(Object *obj, const char *value, return; } - lp_str = g_malloc0(strlen(value)); + lp_str = g_malloc0(strlen(value) + 1); if (!qdev_prop_sanitize_s390x_loadparm(lp_str, value, errp)) { g_free(lp_str); return;
Coverity reports a possible buffer overrun due to a non-NUL-terminated string in scsi_property_set_loadparm(). While things are not so easy, because qdev_prop_sanitize_s390x_loadparm is designed to operate on a buffer that is not NUL-terminated, in this case the string *does* have to be NUL-terminated because it is read by scsi_property_get_loadparm and s390_build_iplb. Cc: jrossi@linux.ibm.com Cc: thuth@redhat.com Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/scsi/scsi-disk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)