On Tue 11.Mar'14 at 13:37:23 -0700, Kees Cook wrote:
On Mon, Mar 10, 2014 at 11:17 PM, Liu ShuoX <[email protected]> wrote:
From: Liu ShuoX <[email protected]>
In case that ramoops_init_przs failed, max_dump_cnt won't be reset to
zero in error handle path.
Signed-off-by: Liu ShuoX <[email protected]>
---
fs/pstore/ram.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 6f96d8c..522e530 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -326,6 +326,7 @@ static void ramoops_free_przs(struct ramoops_context
*cxt)
for (i = 0; !IS_ERR_OR_NULL(cxt->przs[i]); i++)
persistent_ram_free(cxt->przs[i]);
kfree(cxt->przs);
+ cxt->max_dump_cnt = 0;
}
static int ramoops_init_przs(struct device *dev, struct ramoops_context
*cxt,
@@ -350,7 +351,7 @@ static int ramoops_init_przs(struct device *dev, struct
ramoops_context *cxt,
GFP_KERNEL);
if (!cxt->przs) {
dev_err(dev, "failed to initialize a prz array for
dumps\n");
- return -ENOMEM;
+ goto fail_prz;
This will have no effect. If cxt->przs == NULL, ramoops_free_przs will
immediately exit too, not hitting your max_dump_cnt = 0 change.
Perhaps move the =0 in that function to the top before the check and
return?
Yes, you are right. Below has the latest patch which move the =0 to the
top of that function, just as you mentioned. Thanks.
}
for (i = 0; i < cxt->max_dump_cnt; i++) {
@@ -508,7 +509,6 @@ fail_buf:
kfree(cxt->pstore.buf);
fail_clear:
cxt->pstore.bufsize = 0;
- cxt->max_dump_cnt = 0;
fail_cnt:
kfree(cxt->fprz);
fail_init_fprz:
--
1.8.3.2
Otherwise, yes, once fixed, this clean-up looks good -- it keeps the
variable initialization and cleanup all in ramoops_init_przs() which
is how it should be.
Thanks!
-Kees
-----
From: Liu ShuoX <[email protected]>
In case that ramoops_init_przs failed, max_dump_cnt won't be reset to
zero in error handle path.
Signed-off-by: Liu ShuoX <[email protected]>
---
fs/pstore/ram.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 6f96d8c..3b57443 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -320,6 +320,7 @@ static void ramoops_free_przs(struct ramoops_context *cxt)
{
int i;
+ cxt->max_dump_cnt = 0;
if (!cxt->przs)
return;
@@ -350,7 +351,7 @@ static int ramoops_init_przs(struct device *dev, struct ramoops_context *cxt,
GFP_KERNEL);
if (!cxt->przs) {
dev_err(dev, "failed to initialize a prz array for dumps\n");
- return -ENOMEM;
+ goto fail_prz;
}
for (i = 0; i < cxt->max_dump_cnt; i++) {
@@ -508,7 +509,6 @@ fail_buf:
kfree(cxt->pstore.buf);
fail_clear:
cxt->pstore.bufsize = 0;
- cxt->max_dump_cnt = 0;
fail_cnt:
kfree(cxt->fprz);
fail_init_fprz:
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
