On 20-Aug-19 2:16 PM, Harris, James R wrote:


On Aug 20, 2019, at 6:13 AM, Burakov, Anatoly <anatoly.bura...@intel.com> wrote:

On 16-Aug-19 1:13 PM, Jim Harris wrote:
The code checks both rte_mp_request_sync() return
code and that the number of messages in the reply
equals 1.  If rte_mp_request_sync() succeeds but
there was more than one message, those messages
would get leaked.
Found via code review by Anatoly Burakov of patches
that used the vhost code as a template for using
rte_mp_request_sync().
Signed-off-by: Jim Harris <james.r.har...@intel.com>
---
  lib/librte_eal/linux/eal/eal_vfio.c |   16 ++++++++--------
  1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/lib/librte_eal/linux/eal/eal_vfio.c 
b/lib/librte_eal/linux/eal/eal_vfio.c
index 501c74f23..d9541b122 100644
--- a/lib/librte_eal/linux/eal/eal_vfio.c
+++ b/lib/librte_eal/linux/eal/eal_vfio.c
@@ -264,7 +264,7 @@ vfio_open_group_fd(int iommu_group_num)
      int vfio_group_fd;
      char filename[PATH_MAX];
      struct rte_mp_msg mp_req, *mp_rep;
-    struct rte_mp_reply mp_reply;
+    struct rte_mp_reply mp_reply = {0};
      struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
      struct vfio_mp_param *p = (struct vfio_mp_param *)mp_req.param;
  @@ -320,9 +320,9 @@ vfio_open_group_fd(int iommu_group_num)
              RTE_LOG(ERR, EAL, "  bad VFIO group fd\n");
              vfio_group_fd = 0;
          }
-        free(mp_reply.msgs);
      }
  +    free(mp_reply.msgs);

That's not quite correct. This fixes the problem of missing free() when 
nb_received mismatches, but this /adds/ a problem of doing an unnecessary 
free() when rte_mp_request_sync() returns -1. Same for other places, i believe.

This would just resolve to free(NULL) in the -1 case.


Ah, you're right! We did fix that bug :)

With that in mind,

Acked-by: Anatoly Burakov <anatoly.bura...@intel.com>

--
Thanks,
Anatoly

Reply via email to