* [PATCH staging] zcache: fix "zcache=" kernel parameter
@ 2013-07-24 17:04 Bartlomiej Zolnierkiewicz
2013-07-24 18:02 ` Greg KH
2013-07-25 7:39 ` Michal Hocko
0 siblings, 2 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2013-07-24 17:04 UTC (permalink / raw)
To: Greg KH
Cc: devel, konrad.wilk, Cristian Rodríguez, Piotr Sarna,
linux-kernel, Michal Hocko, bob.liu, Kyungmin Park
From: Piotr Sarna <p.sarna@partner.samsung.com>
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.
Signed-off-by: Piotr Sarna <p.sarna@partner.samsung.com>
[bzolnier: updated patch description]
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Bob Liu <bob.liu@oracle.com>
---
drivers/staging/zcache/zcache-main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
index dcceed2..81972fa 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -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");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH staging] zcache: fix "zcache=" kernel parameter
2013-07-24 17:04 [PATCH staging] zcache: fix "zcache=" kernel parameter Bartlomiej Zolnierkiewicz
@ 2013-07-24 18:02 ` Greg KH
2013-07-25 14:00 ` Bartlomiej Zolnierkiewicz
2013-07-25 7:39 ` Michal Hocko
1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2013-07-24 18:02 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: devel, konrad.wilk, Cristian Rodríguez, Piotr Sarna,
linux-kernel, Michal Hocko, bob.liu, Kyungmin Park
On Wed, Jul 24, 2013 at 07:04:14PM +0200, Bartlomiej Zolnierkiewicz wrote:
> From: Piotr Sarna <p.sarna@partner.samsung.com>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH staging] zcache: fix "zcache=" kernel parameter
2013-07-24 18:02 ` Greg KH
@ 2013-07-25 14:00 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2013-07-25 14:00 UTC (permalink / raw)
To: Greg KH
Cc: devel, konrad.wilk, Cristian Rodríguez, Piotr Sarna,
linux-kernel, Michal Hocko, bob.liu, Kyungmin Park
On Wednesday, July 24, 2013 11:02:46 AM Greg KH wrote:
> On Wed, Jul 24, 2013 at 07:04:14PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > From: Piotr Sarna <p.sarna@partner.samsung.com>
> > 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.
> 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.
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH staging] zcache: fix "zcache=" kernel parameter
2013-07-24 17:04 [PATCH staging] zcache: fix "zcache=" kernel parameter Bartlomiej Zolnierkiewicz
2013-07-24 18:02 ` Greg KH
@ 2013-07-25 7:39 ` Michal Hocko
1 sibling, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2013-07-25 7:39 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Greg KH, devel, konrad.wilk, Cristian Rodríguez, Piotr Sarna,
linux-kernel, bob.liu, Kyungmin Park
On Wed 24-07-13 19:04:14, Bartlomiej Zolnierkiewicz wrote:
> From: Piotr Sarna <p.sarna@partner.samsung.com>
> 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.
>
> Signed-off-by: Piotr Sarna <p.sarna@partner.samsung.com>
> [bzolnier: updated patch description]
> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Bob Liu <bob.liu@oracle.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
Thanks!
> ---
> drivers/staging/zcache/zcache-main.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
> index dcceed2..81972fa 100644
> --- a/drivers/staging/zcache/zcache-main.c
> +++ b/drivers/staging/zcache/zcache-main.c
> @@ -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");
> --
> 1.7.9.5
>
>
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] staging: zcache: fix "zcache=" kernel parameter
@ 2013-07-25 10:13 Bartlomiej Zolnierkiewicz
2013-07-25 19:08 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2013-07-25 10:13 UTC (permalink / raw)
To: gregkh
Cc: devel, konrad.wilk, crrodriguez, p.sarna, linux-kernel, mhocko,
bob.liu, kyungmin.park, b.zolnierkie
From: Piotr Sarna <p.sarna@partner.samsung.com>
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.
Signed-off-by: Piotr Sarna <p.sarna@partner.samsung.com>
[bzolnier: updated patch description]
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
Cc: Bob Liu <bob.liu@oracle.com>
---
drivers/staging/zcache/zcache-main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
index dcceed2..81972fa 100644
--- a/drivers/staging/zcache/zcache-main.c
+++ b/drivers/staging/zcache/zcache-main.c
@@ -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");
--
1.8.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] staging: zcache: fix "zcache=" kernel parameter
2013-07-25 10:13 [PATCH] staging: " Bartlomiej Zolnierkiewicz
@ 2013-07-25 19:08 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2013-07-25 19:08 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: devel, konrad.wilk, crrodriguez, p.sarna, linux-kernel, mhocko,
bob.liu, kyungmin.park
On Thu, Jul 25, 2013 at 12:13:54PM +0200, Bartlomiej Zolnierkiewicz wrote:
> From: Piotr Sarna <p.sarna@partner.samsung.com>
<snip>
This whole email is base64, making it impossible for me to add the
needed Cc: stable@ tag to it...
Try it again? It shouldn't be this hard...
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-25 19:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-24 17:04 [PATCH staging] zcache: fix "zcache=" kernel parameter Bartlomiej Zolnierkiewicz
2013-07-24 18:02 ` Greg KH
2013-07-25 14:00 ` Bartlomiej Zolnierkiewicz
2013-07-25 7:39 ` Michal Hocko
-- strict thread matches above, loose matches on Subject: below --
2013-07-25 10:13 [PATCH] staging: " Bartlomiej Zolnierkiewicz
2013-07-25 19:08 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox