* [PATCH] pstore: Tidy up an error check
@ 2020-12-02 6:45 Dan Carpenter
2020-12-02 19:25 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2020-12-02 6:45 UTC (permalink / raw)
To: Kees Cook
Cc: Anton Vorontsov, Colin Cross, Tony Luck, linux-kernel, kernel-janitors
The crypto_alloc_comp() function never returns NULL, it returns error
pointers on error.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
fs/pstore/platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 36714df37d5d..b7a2a2a31dee 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -315,7 +315,7 @@ static void allocate_buf_for_compression(void)
}
ctx = crypto_alloc_comp(zbackend->name, 0, 0);
- if (IS_ERR_OR_NULL(ctx)) {
+ if (IS_ERR(ctx)) {
kfree(buf);
pr_err("crypto_alloc_comp('%s') failed: %ld\n", zbackend->name,
PTR_ERR(ctx));
--
2.29.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] pstore: Tidy up an error check
2020-12-02 6:45 [PATCH] pstore: Tidy up an error check Dan Carpenter
@ 2020-12-02 19:25 ` Kees Cook
2020-12-02 20:00 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2020-12-02 19:25 UTC (permalink / raw)
To: Dan Carpenter
Cc: Anton Vorontsov, Colin Cross, Tony Luck, linux-kernel, kernel-janitors
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@oracle.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.
-Kees
> ---
> fs/pstore/platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 36714df37d5d..b7a2a2a31dee 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -315,7 +315,7 @@ static void allocate_buf_for_compression(void)
> }
>
> ctx = crypto_alloc_comp(zbackend->name, 0, 0);
> - if (IS_ERR_OR_NULL(ctx)) {
> + if (IS_ERR(ctx)) {
> kfree(buf);
> pr_err("crypto_alloc_comp('%s') failed: %ld\n", zbackend->name,
> PTR_ERR(ctx));
> --
> 2.29.2
>
--
Kees Cook
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] pstore: Tidy up an error check
2020-12-02 19:25 ` Kees Cook
@ 2020-12-02 20:00 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-12-02 20:00 UTC (permalink / raw)
To: Kees Cook
Cc: Anton Vorontsov, Colin Cross, Tony Luck, linux-kernel, kernel-janitors
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@oracle.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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-02 20:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 6:45 [PATCH] pstore: Tidy up an error check Dan Carpenter
2020-12-02 19:25 ` Kees Cook
2020-12-02 20:00 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).