Re: [PATCH rdma-next 3/3] RDMA/uverbs: Fix incorrect variable type

From: Jason Gunthorpe <jgg@nvidia.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Leon Romanovsky <leon@kernel.org>,
	Doug Ledford <dledford@redhat.com>,
	Avihai Horon <avihaih@nvidia.com>, <linux-rdma@vger.kernel.org>
Subject: Re: [PATCH rdma-next 3/3] RDMA/uverbs: Fix incorrect variable type
Date: Tue, 8 Dec 2020 10:00:36 -0400	[thread overview]
Message-ID: <20201208140036.GJ552508@nvidia.com> (raw)
In-Reply-To: <20201208075539.GA2789@kadam>

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