lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 2 Dec 2020 23:00:21 +0300 From: Dan Carpenter <dan.carpenter@...cle.com> To: Kees Cook <keescook@...omium.org> Cc: Anton Vorontsov <anton@...msg.org>, Colin Cross <ccross@...roid.com>, Tony Luck <tony.luck@...el.com>, linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org Subject: Re: [PATCH] pstore: Tidy up an error check 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@...cle.com> > > 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
Powered by blists - more mailing lists