David Gibson <[email protected]> writes: > On Thu, Jun 18, 2020 at 08:55:53AM +0200, Markus Armbruster wrote: >> Either I'm confused (quite possible), or kvmppc_check_papr_resize_hpt() >> can leak an Error object on failure. Please walk through the code with >> me: >> >> kvmppc_check_papr_resize_hpt(&resize_hpt_err); >> >> This sets @resize_hpt_err on failure. >> >> if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DEFAULT) { >> /* >> * If the user explicitly requested a mode we should either >> * supply it, or fail completely (which we do below). But if >> * it's not set explicitly, we reset our mode to something >> * that works >> */ >> if (resize_hpt_err) { >> spapr->resize_hpt = SPAPR_RESIZE_HPT_DISABLED; >> error_free(resize_hpt_err); >> resize_hpt_err = NULL; >> >> Case 1: failure and SPAPR_RESIZE_HPT_DEFAULT; we free @resize_hpt_err. >> Good. >> >> } else { >> spapr->resize_hpt = smc->resize_hpt_default; >> } >> } >> >> assert(spapr->resize_hpt != SPAPR_RESIZE_HPT_DEFAULT); >> >> if ((spapr->resize_hpt != SPAPR_RESIZE_HPT_DISABLED) && >> resize_hpt_err) { >> /* >> * User requested HPT resize, but this host can't supply it. >> Bail out >> */ >> error_report_err(resize_hpt_err); >> exit(1); >> >> Case 2: failure and not SPAPR_RESIZE_HPT_DISABLED; fatal. Good. >> >> } >> >> What about case 3: failure and SPAPR_RESIZE_HPT_DISABLED? >> >> Good if we get here via case 1 (we freed @resize_hpt_err). >> >> Else, ??? > > I think you're right, and we leak it in this case - I think I forgot > that in the DISABLED case we still (unnecessarily) ask the kernel if > it can do it. > > Of course, it will only happen once per run, so it's not like it's a > particularly noticeable leak.
Understood. I'll post a patch. Thanks!
