| Message ID | 3b2253d199165648b958570d6c4db86d6ced139d.1699545634.git.maciej.szmigiero@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hv-balloon: avoid alloca() usage | expand |
On 9/11/23 17:02, Maciej S. Szmigiero wrote: > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> > > alloca() is frowned upon, replace it with g_malloc0() + g_autofree. > > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> > --- > hw/hyperv/hv-balloon.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 09.11.23 17:02, Maciej S. Szmigiero wrote: > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> > > alloca() is frowned upon, replace it with g_malloc0() + g_autofree. > Reviewed-by: David Hildenbrand <david@redhat.com> If this fixes a coverity issue of #number, we usually indicate that using "CID: #number" or Fixes: CID: #number"
On 13.11.2023 09:59, David Hildenbrand wrote: > On 09.11.23 17:02, Maciej S. Szmigiero wrote: >> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> >> >> alloca() is frowned upon, replace it with g_malloc0() + g_autofree. >> > > Reviewed-by: David Hildenbrand <david@redhat.com> > > If this fixes a coverity issue of #number, we usually indicate that using "CID: #number" or Fixes: CID: #number" > Will add "CID: #1523903" to the commit message then. Thanks, Maciej
On Mon, 13 Nov 2023 at 08:59, David Hildenbrand <david@redhat.com> wrote: > > On 09.11.23 17:02, Maciej S. Szmigiero wrote: > > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> > > > > alloca() is frowned upon, replace it with g_malloc0() + g_autofree. > > > > Reviewed-by: David Hildenbrand <david@redhat.com> > > If this fixes a coverity issue of #number, we usually indicate that > using "CID: #number" or Fixes: CID: #number" It won't actually fix the CID, though -- the Coverity issue is because Coverity doesn't understand that if you allocate memory for a struct with a single-element array + something extra then it's OK to index off the apparent end of the array because the extra memory is there. Switching the allocation from alloca to g_malloc won't change that, because we're still walking off the end of the defined struct. I don't personally like that coding pattern partly because of this, but I'm assuming we're dealing with somebody else's API here. Using a proper standard variable-length-array rather than a one element array might also help, but again, I'm guessing we don't have that flexibility to change it. thanks -- PMM
diff --git a/hw/hyperv/hv-balloon.c b/hw/hyperv/hv-balloon.c index 66f297c1d7e3..a4b4bde0a1e9 100644 --- a/hw/hyperv/hv-balloon.c +++ b/hw/hyperv/hv-balloon.c @@ -365,7 +365,7 @@ static void hv_balloon_unballoon_posting(HvBalloon *balloon, StateDesc *stdesc) PageRangeTree dtree; uint64_t *dctr; bool our_range; - struct dm_unballoon_request *ur; + g_autofree struct dm_unballoon_request *ur = NULL; size_t ur_size = sizeof(*ur) + sizeof(ur->range_array[0]); PageRange range; bool bret; @@ -387,8 +387,7 @@ static void hv_balloon_unballoon_posting(HvBalloon *balloon, StateDesc *stdesc) assert(dtree.t); assert(dctr); - ur = alloca(ur_size); - memset(ur, 0, ur_size); + ur = g_malloc0(ur_size); ur->hdr.type = DM_UNBALLOON_REQUEST; ur->hdr.size = ur_size; ur->hdr.trans_id = balloon->trans_id; @@ -530,7 +529,7 @@ static void hv_balloon_hot_add_posting(HvBalloon *balloon, StateDesc *stdesc) PageRange *hot_add_range = &balloon->hot_add_range; uint64_t *current_count = &balloon->ha_current_count; VMBusChannel *chan = hv_balloon_get_channel(balloon); - struct dm_hot_add *ha; + g_autofree struct dm_hot_add *ha = NULL; size_t ha_size = sizeof(*ha) + sizeof(ha->range); union dm_mem_page_range *ha_region; uint64_t align, chunk_max_size; @@ -559,9 +558,8 @@ static void hv_balloon_hot_add_posting(HvBalloon *balloon, StateDesc *stdesc) */ *current_count = MIN(hot_add_range->count, chunk_max_size); - ha = alloca(ha_size); + ha = g_malloc0(ha_size); ha_region = &(&ha->range)[1]; - memset(ha, 0, ha_size); ha->hdr.type = DM_MEM_HOT_ADD_REQUEST; ha->hdr.size = ha_size; ha->hdr.trans_id = balloon->trans_id;