On Tue, Dec 08, 2020 at 10:55:39AM +0300, Dan Carpenter wrote: > On Tue, Dec 08, 2020 at 09:35:45AM +0200, Leon Romanovsky wrote: > > @@ -336,19 +335,16 @@ static int UVERBS_HANDLER(UVERBS_METHOD_QUERY_GID_TABLE)( > > attrs, UVERBS_ATTR_QUERY_GID_TABLE_RESP_ENTRIES, > > user_entry_size); > > if (max_entries <= 0) > > - return -EINVAL; > > + return max_entries ?: -EINVAL; > > > > ucontext = ib_uverbs_get_ucontext(attrs); > > if (IS_ERR(ucontext)) > > return PTR_ERR(ucontext); > > ib_dev = ucontext->device; > > > > - if (check_mul_overflow(max_entries, sizeof(*entries), &num_bytes)) > > - return -EINVAL; > > - > > - entries = uverbs_zalloc(attrs, num_bytes); > > - if (!entries) > > - return -ENOMEM; > > + entries = uverbs_kcalloc(attrs, max_entries, sizeof(*entries)); > > + if (IS_ERR(entries)) > > + return PTR_ERR(entries); > > This isn't right. The uverbs_kcalloc() should match every other > kcalloc() function and return NULL on error. This actually buggy > because it returns both is error pointers and NULL so it will lead to > a NULL dereference. It is abnormal, but returing the EOVERFLOW to the caller vs ENOMEM does seem somewhat relevant for debuggability.. If anything on the uverbs_*alloc* path returns NULL it is a bug. Jason
