| Message ID | 1520219524-17160-1-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 5 March 2018 at 03:11, Jason Wang <jasowang@redhat.com> wrote: > The following changes since commit 136c67e07869227b21b3f627316e03679ce7b738: > > Merge remote-tracking branch 'remotes/bkoppelmann/tags/pull-tricore-2018-03-02' into staging (2018-03-02 16:56:20 +0000) > > are available in the git repository at: > > https://github.com/jasowang/qemu.git tags/net-pull-request > > for you to fetch changes up to 46d4d36d0bf2b24b205f2f604f0905db80264eef: > > tap: setting error appropriately when calling net_init_tap_one() (2018-03-05 10:30:16 +0800) > > ---------------------------------------------------------------- > > ---------------------------------------------------------------- > Jay Zhou (1): > tap: setting error appropriately when calling net_init_tap_one() > > Thomas Huth (8): > net: Move error reporting from net_init_client/netdev to the calling site > net: List available netdevs with "-netdev help" > net: Only show vhost-user in the help text if CONFIG_POSIX is defined > net: Make net_client_init() static > net: Remove the deprecated way of dumping network packets > net: Remove the deprecated 'host_net_add' and 'host_net_remove' HMP commands > net: Add a new convenience option "--nic" to configure default/on-board NICs > hw/net: Remove unnecessary header includes > Applied, thanks. -- PMM
On 5 March 2018 at 03:12, Jason Wang <jasowang@redhat.com> wrote: > From: Thomas Huth <thuth@redhat.com> Hi. Coverity (CID 1390615) points out that we leak memory in an error-exit codepath in this function. > +/* For the convenience "--nic" parameter */ > +static int net_param_nic(void *dummy, QemuOpts *opts, Error **errp) > +{ > + char *mac, *nd_id; > + int idx, ret; > + NICInfo *ni; > + const char *type; > + > + type = qemu_opt_get(opts, "type"); > + if (type && g_str_equal(type, "none")) { > + return 0; /* Nothing to do, default_net is cleared in vl.c */ > + } > + > + idx = nic_get_free_idx(); > + if (idx == -1 || nb_nics >= MAX_NICS) { > + error_setg(errp, "no more on-board/default NIC slots available"); > + return -1; > + } > + > + if (!type) { > + qemu_opt_set(opts, "type", "user", &error_abort); > + } > + > + ni = &nd_table[idx]; > + memset(ni, 0, sizeof(*ni)); > + ni->model = qemu_opt_get_del(opts, "model"); > + > + /* Create an ID if the user did not specify one */ > + nd_id = g_strdup(qemu_opts_id(opts)); > + if (!nd_id) { > + nd_id = g_strdup_printf("__org.qemu.nic%i\n", idx); > + qemu_opts_set_id(opts, nd_id); > + } Here we allocate nd_id... > + > + /* Handle MAC address */ > + mac = qemu_opt_get_del(opts, "mac"); > + if (mac) { > + ret = net_parse_macaddr(ni->macaddr.a, mac); > + g_free(mac); > + if (ret) { > + error_setg(errp, "invalid syntax for ethernet address"); > + return -1; > + } > + if (is_multicast_ether_addr(ni->macaddr.a)) { > + error_setg(errp, "NIC cannot have multicast MAC address"); > + return -1; > + } ...but in these two error-exit paths we do not free nd_id. > + } > + qemu_macaddr_default_if_unset(&ni->macaddr); > + > + ret = net_client_init(opts, true, errp); > + if (ret == 0) { > + ni->netdev = qemu_find_netdev(nd_id); > + ni->used = true; > + nb_nics++; > + } Putting a label "out:" here and replacing the 'return -1's with "ret = -1; goto out;" would fix this. > + g_free(nd_id); > + return ret; > +} > + thanks -- PMM
On 2018年04月27日 20:29, Peter Maydell wrote: > On 5 March 2018 at 03:12, Jason Wang <jasowang@redhat.com> wrote: >> From: Thomas Huth <thuth@redhat.com> > Hi. Coverity (CID 1390615) points out that we leak memory > in an error-exit codepath in this function. > >> +/* For the convenience "--nic" parameter */ >> +static int net_param_nic(void *dummy, QemuOpts *opts, Error **errp) >> +{ >> + char *mac, *nd_id; >> + int idx, ret; >> + NICInfo *ni; >> + const char *type; >> + >> + type = qemu_opt_get(opts, "type"); >> + if (type && g_str_equal(type, "none")) { >> + return 0; /* Nothing to do, default_net is cleared in vl.c */ >> + } >> + >> + idx = nic_get_free_idx(); >> + if (idx == -1 || nb_nics >= MAX_NICS) { >> + error_setg(errp, "no more on-board/default NIC slots available"); >> + return -1; >> + } >> + >> + if (!type) { >> + qemu_opt_set(opts, "type", "user", &error_abort); >> + } >> + >> + ni = &nd_table[idx]; >> + memset(ni, 0, sizeof(*ni)); >> + ni->model = qemu_opt_get_del(opts, "model"); >> + >> + /* Create an ID if the user did not specify one */ >> + nd_id = g_strdup(qemu_opts_id(opts)); >> + if (!nd_id) { >> + nd_id = g_strdup_printf("__org.qemu.nic%i\n", idx); >> + qemu_opts_set_id(opts, nd_id); >> + } > Here we allocate nd_id... > >> + >> + /* Handle MAC address */ >> + mac = qemu_opt_get_del(opts, "mac"); >> + if (mac) { >> + ret = net_parse_macaddr(ni->macaddr.a, mac); >> + g_free(mac); >> + if (ret) { >> + error_setg(errp, "invalid syntax for ethernet address"); >> + return -1; >> + } >> + if (is_multicast_ether_addr(ni->macaddr.a)) { >> + error_setg(errp, "NIC cannot have multicast MAC address"); >> + return -1; >> + } > ...but in these two error-exit paths we do not free nd_id. > >> + } >> + qemu_macaddr_default_if_unset(&ni->macaddr); >> + >> + ret = net_client_init(opts, true, errp); >> + if (ret == 0) { >> + ni->netdev = qemu_find_netdev(nd_id); >> + ni->used = true; >> + nb_nics++; >> + } > Putting a label "out:" here and replacing the 'return -1's > with "ret = -1; goto out;" would fix this. > >> + g_free(nd_id); >> + return ret; >> +} >> + > thanks > -- PMM > Thanks Peter. Thomas, want to send a fix for this?
The following changes since commit 136c67e07869227b21b3f627316e03679ce7b738: Merge remote-tracking branch 'remotes/bkoppelmann/tags/pull-tricore-2018-03-02' into staging (2018-03-02 16:56:20 +0000) are available in the git repository at: https://github.com/jasowang/qemu.git tags/net-pull-request for you to fetch changes up to 46d4d36d0bf2b24b205f2f604f0905db80264eef: tap: setting error appropriately when calling net_init_tap_one() (2018-03-05 10:30:16 +0800) ---------------------------------------------------------------- ---------------------------------------------------------------- Jay Zhou (1): tap: setting error appropriately when calling net_init_tap_one() Thomas Huth (8): net: Move error reporting from net_init_client/netdev to the calling site net: List available netdevs with "-netdev help" net: Only show vhost-user in the help text if CONFIG_POSIX is defined net: Make net_client_init() static net: Remove the deprecated way of dumping network packets net: Remove the deprecated 'host_net_add' and 'host_net_remove' HMP commands net: Add a new convenience option "--nic" to configure default/on-board NICs hw/net: Remove unnecessary header includes hmp-commands.hx | 30 ------ hmp.h | 3 - hw/net/e1000.c | 1 - hw/net/lance.c | 3 - hw/net/ne2000.c | 2 - hw/net/pcnet-pci.c | 1 - hw/net/pcnet.c | 1 - hw/net/rtl8139.c | 2 - hw/net/xgmac.c | 1 - include/net/net.h | 4 +- include/net/vhost_net.h | 3 + include/sysemu/sysemu.h | 1 + monitor.c | 61 ------------ net/dump.c | 102 +-------------------- net/net.c | 239 +++++++++++++++++++++++------------------------- net/tap.c | 22 ++++- qapi/net.json | 29 ++---- qemu-doc.texi | 16 ---- qemu-options.hx | 48 +++++++--- tests/test-hmp.c | 2 - vl.c | 10 +- 21 files changed, 190 insertions(+), 391 deletions(-)