| Message ID | 1455717855-2344-1-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Ian Campbell writes ("[PATCH 1/3] xl: free config_data on error in domain_create"):
> CID: 1055898
...
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index d5a397a..e819ee6 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2795,6 +2795,7 @@ static uint32_t create_domain(struct domain_create *dom_info)
> if (!restoring && extra_config && strlen(extra_config)) {
> if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
> fprintf(stderr, "Failed to attach extra configuration\n");
> + free(config_data);
> return ERROR_FAIL;
This LGTM.
> /* allocate space for the extra config plus two EOLs plus \0 */
> @@ -6525,6 +6526,7 @@ int main_networkattach(int argc, char **argv)
> }
>
> xlu_cfg_destroy(config);
> + config = NULL;
>
> if (argc > 0) return 1; /* Parse error above */
But this doesn't apply and the current code in main_networkattach
doesn't seem to have a bug that something this would fix.
Ian.
On Fri, 2016-02-19 at 16:02 +0000, Ian Jackson wrote: > Ian Campbell writes ("[PATCH 1/3] xl: free config_data on error in > domain_create"): > > CID: 1055898 > ... > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > index d5a397a..e819ee6 100644 > > --- a/tools/libxl/xl_cmdimpl.c > > +++ b/tools/libxl/xl_cmdimpl.c > > @@ -2795,6 +2795,7 @@ static uint32_t create_domain(struct > > domain_create *dom_info) > > if (!restoring && extra_config && strlen(extra_config)) { > > if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) > > { > > fprintf(stderr, "Failed to attach extra > > configuration\n"); > > + free(config_data); > > return ERROR_FAIL; > > This LGTM. > > > /* allocate space for the extra config plus two EOLs plus > > \0 */ > > @@ -6525,6 +6526,7 @@ int main_networkattach(int argc, char **argv) > > } > > > > xlu_cfg_destroy(config); > > + config = NULL; > > > > if (argc > 0) return 1; /* Parse error above */ > > But this doesn't apply and the current code in main_networkattach > doesn't seem to have a bug that something this would fix. Looks like a stray line which should have been in "xl: network-attach: free config object after use", sorry. I'll wait for you to push the batch you have in hand and then rebase fixing this as I go. Ian.
On Fri, 2016-02-19 at 16:08 +0000, Ian Campbell wrote: > /* allocate space for the extra config plus two EOLs > > > plus > > > \0 */ > > > @@ -6525,6 +6526,7 @@ int main_networkattach(int argc, char **argv) > > > } > > > > > > xlu_cfg_destroy(config); > > > + config = NULL; > > > > > > if (argc > 0) return 1; /* Parse error above */ > > > > But this doesn't apply and the current code in main_networkattach > > doesn't seem to have a bug that something this would fix. > > Looks like a stray line which should have been in "xl: network-attach: free > config object after use", sorry. Which I see I somehow did send out! > I'll wait for you to push the batch you have in hand and then rebase fixing > this as I go. I'll still take this approach. Ian.
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index d5a397a..e819ee6 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -2795,6 +2795,7 @@ static uint32_t create_domain(struct domain_create *dom_info) if (!restoring && extra_config && strlen(extra_config)) { if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) { fprintf(stderr, "Failed to attach extra configuration\n"); + free(config_data); return ERROR_FAIL; } /* allocate space for the extra config plus two EOLs plus \0 */ @@ -6525,6 +6526,7 @@ int main_networkattach(int argc, char **argv) } xlu_cfg_destroy(config); + config = NULL; if (argc > 0) return 1; /* Parse error above */
CID: 1055898 Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- tools/libxl/xl_cmdimpl.c | 2 ++ 1 file changed, 2 insertions(+)