Skip to contentSkip to conversation

zcache: fix "zcache=" kernel parameter

PATCH In review Bartlomiej Zolnierkiewicz 3 participants 4 messages Jul 24, 2013 – Jul 25, 2013 Staging

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 zcache: fix "zcache=" kernel parameter

From: Piotr Sarna <[email protected]>
Subject: [PATCH] zcache: fix "zcache=" kernel parameter

Commit 835f2f5 ("staging: zcache: enable zcache to be built/loaded as a module") introduced an incorrect handling of "zcache=" parameter.

Inside zcache_comp_init() function, zcache_comp_name variable is checked for being empty. If not empty, the above variable is tested for being compatible with Crypto API. Unfortunately, after that function ends unconditionally (by the "goto out" directive) and returns:
- non-zero value if verification succeeded, wrongly indicating an error
- zero value if verification failed, falsely informing that function zcache_comp_init() ended properly.

A solution to this problem is as following:
1. Move the "goto out" directive inside the "if (!ret)" statement
2. In case that crypto_has_comp() returned 0, change the value of ret to non-zero before "goto out" to indicate an error.

This patch replaces an earlier one from Michal Hocko (based on report from Cristian Rodríguez):

	http://permalink.gmane.org/gmane.linux.kernel.mm/102484

It also addressed the same issue but didn't fix the zcache_comp_init() for case when the compressor data passed to "zcache=" option was invalid or unsupported.

[bzolnier: updated patch description]
Cc Cristian Rodríguez <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Bob Liu <[email protected]>
---
 drivers/staging/zcache/zcache-main.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/zcache/zcache-main.c +4 -2
index dcceed2..81972fa 100644
@@ -1811,10 +1811,12 @@ static int zcache_comp_init(void)
 #else
 	if (*zcache_comp_name != '\0') {
 		ret = crypto_has_comp(zcache_comp_name, 0, 0);
-		if (!ret)
+		if (!ret) {
 			pr_info("zcache: %s not supported\n",
 					zcache_comp_name);
-		goto out;
+			ret = 1;
+			goto out;
+		}
 	}
 	if (!ret)
 		strcpy(zcache_comp_name, "lzo");
Signature · 1 line
1.7.9.5
maintainer
#1 lore raw
Quoted 2 lines · Bartlomiej Zolnierkiewicz

From: Piotr Sarna <[email protected]>
Subject: [PATCH] zcache: fix "zcache=" kernel parameter

What's with the duplicated Subject: line here? Why? That means the
From: line there will probably not get recognized by git (well, it might), but then the subject is duplicated again, just like Linus has complained about not doing in the past.

And in a mime-encoded format? Ugh, please fix your email client and send it again (yes, git can handle it, but it's a pain at times...)

third time's a charm?

greg k-h

#2 lore raw
Quoted 8 lines · Greg KH
Quoted 2 lines · Bartlomiej Zolnierkiewicz

From: Piotr Sarna <[email protected]>
Subject: [PATCH] zcache: fix "zcache=" kernel parameter

What's with the duplicated Subject: line here? Why? That means the
From: line there will probably not get recognized by git (well, it might), but then the subject is duplicated again, just like Linus has complained about not doing in the past.

Uh, sorry about that.

Quoted 2 lines

And in a mime-encoded format? Ugh, please fix your email client and send it again (yes, git can handle it, but it's a pain at times...)

This seems to be some bug in my version of KMail (it gets confused on "í" character). I've used git send-email this time.

Quoted 1 line

third time's a charm?

I hope that the patch is OK now (BTW I applied Michal's ACK that come in the meantime so there is some positive side of the delay ;).

Once again sorry for the bad patch format and thanks for the patience.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Acked-by on v1 1/1
#3 lore raw
In reply to Bartlomiej Zolnierkiewicz
Quoted 28 lines · Bartlomiej Zolnierkiewicz

From: Piotr Sarna <[email protected]>
Subject: [PATCH] zcache: fix "zcache=" kernel parameter

Commit 835f2f5 ("staging: zcache: enable zcache to be built/loaded as a module") introduced an incorrect handling of "zcache=" parameter.

Inside zcache_comp_init() function, zcache_comp_name variable is checked for being empty. If not empty, the above variable is tested for being compatible with Crypto API. Unfortunately, after that function ends unconditionally (by the "goto out" directive) and returns:
- non-zero value if verification succeeded, wrongly indicating an error
- zero value if verification failed, falsely informing that function zcache_comp_init() ended properly.

A solution to this problem is as following:
1. Move the "goto out" directive inside the "if (!ret)" statement
2. In case that crypto_has_comp() returned 0, change the value of ret to non-zero before "goto out" to indicate an error.

This patch replaces an earlier one from Michal Hocko (based on report from Cristian Rodríguez):

	http://permalink.gmane.org/gmane.linux.kernel.mm/102484

It also addressed the same issue but didn't fix the zcache_comp_init() for case when the compressor data passed to "zcache=" option was invalid or unsupported.

Quoted 8 lines
[bzolnier: updated patch description]
Cc Cristian Rodríguez <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Bob Liu <[email protected]>

Thanks!

Quoted 28 lines
---
 drivers/staging/zcache/zcache-main.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/zcache/zcache-main.c +4 -2
index dcceed2..81972fa 100644
@@ -1811,10 +1811,12 @@ static int zcache_comp_init(void)
 #else
 	if (*zcache_comp_name != '\0') {
 		ret = crypto_has_comp(zcache_comp_name, 0, 0);
-		if (!ret)
+		if (!ret) {
 			pr_info("zcache: %s not supported\n",
 					zcache_comp_name);
-		goto out;
+			ret = 1;
+			goto out;
+		}
 	}
 	if (!ret)
 		strcpy(zcache_comp_name, "lzo");
Signature · 1 line
1.7.9.5
Signature · 2 lines
Michal Hocko
SUSE Labs