On Wed, Dec 02, 2020 at 11:25:46AM -0800, Kees Cook wrote: > On Wed, Dec 02, 2020 at 09:45:31AM +0300, Dan Carpenter wrote: > > The crypto_alloc_comp() function never returns NULL, it returns error > > pointers on error. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > I replied to an identical patch yesterday, actually: > https://lore.kernel.org/lkml/202012011215.B9BF24A6D@keescook/ > > Using IS_ERR_OR_NULL() is more robust, and this isn't fast path, so I'd > prefer to keep it that way. > The NULL return doesn't make any sense though because crypto_alloc_comp() isn't optional... When a function returns both error pointers and NULLs then the NULL is special kind of success. p = get_feature(); If "p" is an error pointer that means an error happened. If "p" is NULL that means the feature is disabled in the .config or whatever. We can't return a valid pointer because the feature doesn't exist but it's also not an error so it doesn't return an error pointer. The code should not print a warning, maybe an info level printk at most. Then the driver should continue operating with the feature turned off. Two of the callers for crypto_alloc_comp() check for error pointers and NULL and three only check for error pointers. It's inconsistent. regards, dan carpenter
