linux-ext4 - [patch] ext4 crypto: testing the wrong variable

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 8 Apr 2015 11:53:38 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	"Theodore Ts'o" <tytso@....edu>,
	Michael Halcrow <mhalcrow@...gle.com>
Cc:	Andreas Dilger <adilger.kernel@...ger.ca>,
	linux-ext4@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: [patch] ext4 crypto: testing the wrong variable

The intention was to check "IS_ERR(*buf)" instead of "IS_ERR(buf)".
It's never an ERR_PTR() in the current code so this is a harmless
mistake.

Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
---
I have a static checker warning for when people check IS_ERR() and it's
not an error pointer.  Adding these extra checks introduces deliberate
warnings and makes my work harder which means that bugs are missed.

Also there is no good reason to pass an ERR_PTR to
ext4_fname_crypto_free_buffer() but eventually someone will take
advantage of this feature to write ugly code.  Ugly code is going to be
buggy.

At least do a "if (WARN_ON(IS_ERR(*buf))) return;".  I could filter out
impossible conditions which are inside a WARN_ON() and it will mean
people don't actually pass error pointers here.

diff --git a/fs/ext4/crypto_fname.c b/fs/ext4/crypto_fname.c
index 5fda403..f076b52 100644
--- a/fs/ext4/crypto_fname.c
+++ b/fs/ext4/crypto_fname.c
@@ -558,7 +558,7 @@ int ext4_fname_crypto_alloc_buffer(struct ext4_fname_crypto_ctx *ctx,
  */
 void ext4_fname_crypto_free_buffer(void **buf)
 {
-	if (*buf == NULL || IS_ERR(buf))
+	if (*buf == NULL || IS_ERR(*buf))
 		return;
 	kfree(*buf);
 	*buf = NULL;
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html