On 3/19/21 8:52 AM, Borislav Petkov wrote: > On Fri, Mar 19, 2021 at 05:22:56PM +0200, Jarkko Sakkinen wrote: >> I did misread it for the first time. >> >> So let's sanity: you *are* going to squash the patches together because >> that way it's factors easier to backport the whole thing? >> >> Is this the correct understanding? > I squashed Kai's fix because I don't want to break people's bisection if > they land between your patch and his fix. They're already troubled enough > chasing an issue, don't want to have them get a NULL ptr in sgx land. > > Now, looking at dhansen's fix: what can happen if nid is uninitialized? > AFAICT, we'll end up in > > static inline int __next_node(int n, const nodemask_t *srcp) > { > return min_t(int,MAX_NUMNODES,find_next_bit(srcp->bits, MAX_NUMNODES, n+1)); > } > > with n uninitialized and depending on its value it'll either return > MAX_NUMNODES so we'll try to allocate on the first node or try to > allocate on some other node. > > Now, if you think that that is still problematic enough for enclave > creation, then I'll fold his patch too. > > So yes, the main reason is usability and not breaking bisection. > > So, what would you prefer? It's probably best to squash my patch in. The uninitialized value could *theoretically* cause the search to start at the wrong node and then end before every node has been visited. That could cause premature allocation failures. But, I seriously doubt anyone will notice either way.