On Mon, May 11, 2020 at 02:26:30PM -0700, Divya Indi wrote: > >> @@ -1123,6 +1156,18 @@ int ib_nl_handle_resolve_resp(struct sk_buff *skb, > >> > >> send_buf = query->mad_buf; > >> > >> + /* > >> + * Make sure the IB_SA_NL_QUERY_SENT flag is set before > >> + * processing this query. If flag is not set, query can be accessed in > >> + * another context while setting the flag and processing the query will > >> + * eventually release it causing a possible use-after-free. > >> + */ > > This comment doesn't really make sense, flags insige the memory being > > freed inherently can't prevent use after free. > > I can definitely re-phrase here to make things clearer. But, the idea here is > in the unlikely/rare case where a response for a query comes in before the flag has been > set in ib_nl_make_request, we want to wait for the flag to be sent before proceeding. > The response handler will eventually release the query so this wait avoids that if the flag has not been set > else > "query->flags |= IB_SA_NL_QUERY_SENT;" > will be accessing a query which was freed due to the above mentioned race. > > It is unlikely since getting a response => We have actually sent out the query to ibacm. > > How about this - > > "Getting a response is indicative of having sent out the query, but in an unlikely race when > the response comes in before setting IB_SA_NL_QUERY_SENT, we need to wait till the flag is set to > avoid accessing a query that has been released." It still makes no sense, a flag that is set before freeing the memory is fundamentally useless to prevent races. Jason
