[PATCH 0/2] mkfs.ubifs: some error handling fixes

[PATCH 0/2] mkfs.ubifs: some error handling fixes

[David Oberhollenzer]

Hi,

poking around the mkfs.ubifs source, I discovered two cases of what I
suspect to be error cases that got ignored instead of propperly handled.

The first is probably the more delicate one where the return value of
gen_essiv_salt is never checked in the fscrypt code.

The second one concerns pre-populating the ubifs image, where a readdir
loop is aborted if readdir fails, but the error status is never reported
back and the code simply marches on.

Please tell me if my fixes break something elsewhere or my assumptions
about the intended behaviour are flawed. Otherwise I will merge this some
time next week.

Thanks,

David


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/2] mkfs.ubifs: fscrypt: bail from encrypt_block if gen_essiv_salt fails

[David Oberhollenzer]

What originally cought my attention was that gen_essiv_salt has a
size_t return type and error paths that return -1 on failure.
Further investigation revealed that the error value is never checked
for. The encrypt_block function doesn't use the return value in any
way and simply continues onward.

Furthermore, the gen_essiv_salt function has an error case that emits
an error message but returns success state.

This patch modifes gen_essiv_salt to return an error status in all
error branches, changes the return type to ssize_t and adds a check
to encrypt_block if gen_essiv_salt fails.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/crypto.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/ubifs-utils/mkfs.ubifs/crypto.c b/ubifs-utils/mkfs.ubifs/crypto.c
index cd68e37..d31bd2a 100644
--- a/ubifs-utils/mkfs.ubifs/crypto.c
+++ b/ubifs-utils/mkfs.ubifs/crypto.c
@@ -109,7 +109,7 @@ fail:
 	return -1;
 }
 
-static size_t gen_essiv_salt(const void *iv, size_t iv_len, const void *key, size_t key_len, void *salt)
+static ssize_t gen_essiv_salt(const void *iv, size_t iv_len, const void *key, size_t key_len, void *salt)
 {
 	size_t ret;
 	const EVP_CIPHER *cipher;
@@ -127,8 +127,10 @@ static size_t gen_essiv_salt(const void *iv, size_t iv_len, const void *key, siz
 	}
 
 	ret = do_encrypt(cipher, iv, iv_len, sha256, EVP_MD_size(EVP_sha256()), NULL, 0, salt);
-	if (ret != iv_len)
+	if (ret != iv_len) {
 		errmsg("Unable to compute ESSIV salt, return value %zi instead of %zi", ret, iv_len);
+		return -1;
+	}
 
 	free(sha256);
 
@@ -154,7 +156,8 @@ static ssize_t encrypt_block(const void *plaintext, size_t size,
 
 	if (cipher == EVP_aes_128_cbc()) {
 		tweak = alloca(ivsize);
-		gen_essiv_salt(&iv, FS_IV_SIZE, key, key_len, tweak);
+		if (gen_essiv_salt(&iv, FS_IV_SIZE, key, key_len, tweak) < 0)
+			return -1;
 	} else {
 		tweak = &iv;
 	}
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/2] mkfs.ubifs: abort add_directory if readdir fails

[David Oberhollenzer]

The existing code sets 'err' to -1 and breaks the readdir loop, but
the error state is never read. This patch modifies the readdir loop
to actualy jump to the error handling branch if readdir fails.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index 5748aaa..72ae1b4 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -2043,8 +2043,7 @@ static int add_directory(const char *dir_name, ino_t dir_inum, struct stat *st,
 			if (errno == 0)
 				break;
 			sys_err_msg("error reading directory '%s'", dir_name);
-			err = -1;
-			break;
+			goto out_free;
 		}
 
 		if (strcmp(".", entry->d_name) == 0)
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/