Den lör 21 juli 2018 kl 14:58 skrev Taehee Yoo <ap420...@gmail.com>: > > 2018-07-21 2:18 GMT+09:00 Martin KaFai Lau <ka...@fb.com>: > > On Sat, Jul 21, 2018 at 01:04:45AM +0900, Taehee Yoo wrote: > >> rhashtable_lookup() can return NULL. so that NULL pointer > >> check routine should be added. > >> > >> Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY") > >> Signed-off-by: Taehee Yoo <ap420...@gmail.com> > >> --- > >> net/core/xdp.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/net/core/xdp.c b/net/core/xdp.c > >> index 9d1f220..1c12bc7 100644 > >> --- a/net/core/xdp.c > >> +++ b/net/core/xdp.c > >> @@ -345,7 +345,8 @@ static void __xdp_return(void *data, struct > >> xdp_mem_info *mem, bool napi_direct, > >> rcu_read_lock(); > >> /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() > >> */ > >> xa = rhashtable_lookup(mem_id_ht, &mem->id, > >> mem_id_rht_params); > >> - xa->zc_alloc->free(xa->zc_alloc, handle); > >> + if (xa) > >> + xa->zc_alloc->free(xa->zc_alloc, handle); > > hmm...It is not clear to me the "!xa" case don't have to be handled? > > Thank you for reviewing! > > Returning NULL pointer is bug case such as calling after use > xdp_rxq_info_unreg(). > so that, I think it can't handle at that moment. > we can make __xdp_return to add WARN_ON_ONCE() or > add return error code to driver. > But I'm not sure if these is useful information. > > I might have misunderstood scenario of MEM_TYPE_ZERO_COPY > because there is no use case of MEM_TYPE_ZERO_COPY yet. >
Taehee, again, sorry for the slow response and thanks for patch! If xa is NULL, the driver has a buggy/broken implementation. What would be a proper way of dealing with this? BUG? Björn > Thanks! > > > > >> rcu_read_unlock(); > >> default: > >> /* Not possible, checked in xdp_rxq_info_reg_mem_model() */ > >> -- > >> 2.9.3 > >>