| Message ID | c7e801a6ff84755e97f429656f7beb997242d6c9.1402720673.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Jun 14, 2014 at 12:48:57PM +0800, Hu Tao wrote: > If pc-dimm is specified on qemu command line, but only with > -m size (aka not -m size,maxmem,slots) then qemu will core dump. > > This patch fixes the problem. > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > hw/mem/pc-dimm.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > index 8c26568..6e8bf43 100644 > --- a/hw/mem/pc-dimm.c > +++ b/hw/mem/pc-dimm.c > @@ -107,7 +107,12 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, > uint64_t new_addr, ret = 0; > uint64_t address_space_end = address_space_start + address_space_size; > > - assert(address_space_end > address_space_size); > + if (address_space_size == 0) { > + error_setg(errp, "can't add memory beyond 0x%" PRIx64, > + address_space_end); That's quite an unfriendly error message, isn't it? Why not explain what the problem is to the user? > + goto out; > + } > + I would move the assert to this point. It protects against integer overflow. > object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list); > > if (hint) { > -- > 1.9.3
On Sun, Jun 15, 2014 at 11:53:52AM +0300, Michael S. Tsirkin wrote: > On Sat, Jun 14, 2014 at 12:48:57PM +0800, Hu Tao wrote: > > If pc-dimm is specified on qemu command line, but only with > > -m size (aka not -m size,maxmem,slots) then qemu will core dump. > > > > This patch fixes the problem. > > > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > hw/mem/pc-dimm.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > > index 8c26568..6e8bf43 100644 > > --- a/hw/mem/pc-dimm.c > > +++ b/hw/mem/pc-dimm.c > > @@ -107,7 +107,12 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, > > uint64_t new_addr, ret = 0; > > uint64_t address_space_end = address_space_start + address_space_size; > > > > - assert(address_space_end > address_space_size); > > + if (address_space_size == 0) { > > + error_setg(errp, "can't add memory beyond 0x%" PRIx64, > > + address_space_end); > > That's quite an unfriendly error message, isn't it? > Why not explain what the problem is to the user? Thanks! patch updated. > > > + goto out; > > + } > > + > > I would move the assert to this point. It protects against > integer overflow. > > > object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list); > > > > if (hint) { > > > > -- > > 1.9.3
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 8c26568..6e8bf43 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -107,7 +107,12 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, uint64_t new_addr, ret = 0; uint64_t address_space_end = address_space_start + address_space_size; - assert(address_space_end > address_space_size); + if (address_space_size == 0) { + error_setg(errp, "can't add memory beyond 0x%" PRIx64, + address_space_end); + goto out; + } + object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list); if (hint) {
If pc-dimm is specified on qemu command line, but only with -m size (aka not -m size,maxmem,slots) then qemu will core dump. This patch fixes the problem. Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- hw/mem/pc-dimm.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)