Messages in this thread | | | Date | Thu, 18 Oct 2012 09:31:49 -0400 | From | Vivek Goyal <> | Subject | Re: [PATCH] Fix use-after-free of q->root_blkg and q->root_rl.blkg |
| |
On Thu, Oct 18, 2012 at 11:56:34AM +0900, Jun'ichi Nomura wrote:
[..] > >>> if (ent == &q->root_blkg->q_node) > >> > >> So ent is not &q->root_blkg->q_node. > > > > If q->root_blkg is NULL, will it not lead to NULL pointer dereference. > > (q->root_blkg->q_node). > > It's not dereferenced.
Ok. We are taking address of root_blkg->q_node so even if root_blkg=NULL, address is just offset from null. Little subtle for me. :-)
> > >>> ent = ent->next; > >>> if (ent == &q->blkg_list) > >>> return NULL; > >> > >> And we return NULL here. > >> > >> Ah, yes. You are correct. > >> We can do without the above hunk. > > > > I would rather prefer to check for this boundary condition early and > > return instead of letting it fall through all these conditions and > > then figure out yes we have no next rl. IMO, code becomes easier to > > understand if nothing else. Otherwise one needs a step by step > > explanation as above to show that case of q->root_blkg is covered. > > I have same opinion as yours that it's good for readability.
Tejun, for the sake of readability, are you fine with keeping the original check and original patch which I had acked.
Thanks Vivek
| |