* [PATCH 1/3] pstore: Delete six error messages for a failed memory allocation
2017-08-16 19:20 [PATCH 0/3] pstore: Adjustments for some function implementations SF Markus Elfring
@ 2017-08-16 19:21 ` SF Markus Elfring
2017-08-16 19:32 ` Kees Cook
2017-08-16 19:22 ` [PATCH 2/3] pstore: Improve a size determination in three functions SF Markus Elfring
2017-08-16 19:23 ` [PATCH 3/3] pstore: Adjust two checks for null pointers SF Markus Elfring
2 siblings, 1 reply; 9+ messages in thread
From: SF Markus Elfring @ 2017-08-16 19:21 UTC (permalink / raw)
To: Anton Vorontsov, Colin Cross, Kees Cook, Tony Luck,
kernel-janitors; +Cc: LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 16 Aug 2017 20:30:44 +0200
Omit extra messages for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
fs/pstore/ram.c | 6 +-----
fs/pstore/ram_core.c | 15 ++++-----------
2 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 7125b398d312..42d27e5fac9f 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -719,7 +719,6 @@ static int ramoops_probe(struct platform_device *pdev)
if (dev_of_node(dev) && !pdata) {
pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata) {
- pr_err("cannot allocate platform data buffer\n");
err = -ENOMEM;
goto fail_out;
}
@@ -814,7 +813,6 @@ static int ramoops_probe(struct platform_device *pdev)
cxt->pstore.bufsize = max(cxt->record_size, cxt->pstore.bufsize);
cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
if (!cxt->pstore.buf) {
- pr_err("cannot allocate pstore buffer\n");
err = -ENOMEM;
goto fail_clear;
}
@@ -904,10 +902,8 @@ static void ramoops_register_dummy(void)
pr_info("using module parameters\n");
dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL);
- if (!dummy_data) {
- pr_info("could not allocate pdata\n");
+ if (!dummy_data)
return;
- }
dummy_data->mem_size = mem_size;
dummy_data->mem_address = mem_address;
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index e11672aa4575..fafa8af1289c 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -292,10 +292,8 @@ void persistent_ram_save_old(struct persistent_ram_zone *prz)
if (!prz->old_log) {
persistent_ram_ecc_old(prz);
prz->old_log = kmalloc(size, GFP_KERNEL);
- }
- if (!prz->old_log) {
- pr_err("failed to allocate buffer\n");
- return;
+ if (!prz->old_log)
+ return;
}
prz->old_log_size = size;
@@ -411,8 +409,5 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size,
- if (!pages) {
- pr_err("%s: Failed to allocate array for %u pages\n",
- __func__, page_count);
+ if (!pages)
return NULL;
- }
for (i = 0; i < page_count; i++) {
phys_addr_t addr = page_start + i * PAGE_SIZE;
@@ -529,7 +524,5 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
- if (!prz) {
- pr_err("failed to allocate persistent ram zone\n");
+ if (!prz)
goto err;
- }
/* Initialize general buffer state. */
raw_spin_lock_init(&prz->buffer_lock);
--
2.14.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] pstore: Delete six error messages for a failed memory allocation
2017-08-16 19:21 ` [PATCH 1/3] pstore: Delete six error messages for a failed memory allocation SF Markus Elfring
@ 2017-08-16 19:32 ` Kees Cook
2017-08-16 19:58 ` SF Markus Elfring
0 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2017-08-16 19:32 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Anton Vorontsov, Colin Cross, Tony Luck, kernel-janitors, LKML
On Wed, Aug 16, 2017 at 12:21 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 16 Aug 2017 20:30:44 +0200
>
> Omit extra messages for a memory allocation failure in these functions.
I'm not interested in removing these since they provide additional
details about which specific allocation failed.
> This issue was detected by using the Coccinelle software.
For all these kinds of patches please include details on which
coccinelle script did the detection...
-Kees
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> fs/pstore/ram.c | 6 +-----
> fs/pstore/ram_core.c | 15 ++++-----------
> 2 files changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 7125b398d312..42d27e5fac9f 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -719,7 +719,6 @@ static int ramoops_probe(struct platform_device *pdev)
> if (dev_of_node(dev) && !pdata) {
> pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> if (!pdata) {
> - pr_err("cannot allocate platform data buffer\n");
> err = -ENOMEM;
> goto fail_out;
> }
> @@ -814,7 +813,6 @@ static int ramoops_probe(struct platform_device *pdev)
> cxt->pstore.bufsize = max(cxt->record_size, cxt->pstore.bufsize);
> cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL);
> if (!cxt->pstore.buf) {
> - pr_err("cannot allocate pstore buffer\n");
> err = -ENOMEM;
> goto fail_clear;
> }
> @@ -904,10 +902,8 @@ static void ramoops_register_dummy(void)
> pr_info("using module parameters\n");
>
> dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL);
> - if (!dummy_data) {
> - pr_info("could not allocate pdata\n");
> + if (!dummy_data)
> return;
> - }
>
> dummy_data->mem_size = mem_size;
> dummy_data->mem_address = mem_address;
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index e11672aa4575..fafa8af1289c 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -292,10 +292,8 @@ void persistent_ram_save_old(struct persistent_ram_zone *prz)
> if (!prz->old_log) {
> persistent_ram_ecc_old(prz);
> prz->old_log = kmalloc(size, GFP_KERNEL);
> - }
> - if (!prz->old_log) {
> - pr_err("failed to allocate buffer\n");
> - return;
> + if (!prz->old_log)
> + return;
> }
>
> prz->old_log_size = size;
> @@ -411,8 +409,5 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size,
> - if (!pages) {
> - pr_err("%s: Failed to allocate array for %u pages\n",
> - __func__, page_count);
> + if (!pages)
> return NULL;
> - }
>
> for (i = 0; i < page_count; i++) {
> phys_addr_t addr = page_start + i * PAGE_SIZE;
> @@ -529,7 +524,5 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
> - if (!prz) {
> - pr_err("failed to allocate persistent ram zone\n");
> + if (!prz)
> goto err;
> - }
>
> /* Initialize general buffer state. */
> raw_spin_lock_init(&prz->buffer_lock);
> --
> 2.14.0
>
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: pstore: Delete six error messages for a failed memory allocation
2017-08-16 19:32 ` Kees Cook
@ 2017-08-16 19:58 ` SF Markus Elfring
0 siblings, 0 replies; 9+ messages in thread
From: SF Markus Elfring @ 2017-08-16 19:58 UTC (permalink / raw)
To: Kees Cook; +Cc: Anton Vorontsov, Colin Cross, Tony Luck, kernel-janitors, LKML
> I'm not interested in removing these since they provide additional
> details about which specific allocation failed.
Do you find the default allocation failure report insufficient?
Regards,
Markus
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] pstore: Improve a size determination in three functions
2017-08-16 19:20 [PATCH 0/3] pstore: Adjustments for some function implementations SF Markus Elfring
2017-08-16 19:21 ` [PATCH 1/3] pstore: Delete six error messages for a failed memory allocation SF Markus Elfring
@ 2017-08-16 19:22 ` SF Markus Elfring
2017-08-16 19:33 ` Kees Cook
2017-08-16 19:23 ` [PATCH 3/3] pstore: Adjust two checks for null pointers SF Markus Elfring
2 siblings, 1 reply; 9+ messages in thread
From: SF Markus Elfring @ 2017-08-16 19:22 UTC (permalink / raw)
To: Anton Vorontsov, Colin Cross, Kees Cook, Tony Luck,
kernel-janitors; +Cc: LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 16 Aug 2017 20:50:15 +0200
Replace the specification of data types by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
fs/pstore/ram.c | 3 +--
fs/pstore/ram_core.c | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 42d27e5fac9f..0ef95c384bed 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -293,8 +293,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
*/
struct persistent_ram_zone *tmp_prz, *prz_next;
- tmp_prz = kzalloc(sizeof(struct persistent_ram_zone),
- GFP_KERNEL);
+ tmp_prz = kzalloc(sizeof(*tmp_prz), GFP_KERNEL);
if (!tmp_prz)
return -ENOMEM;
free_prz = true;
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index fafa8af1289c..5d9f7280d757 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -405,7 +405,7 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size,
else
prot = pgprot_writecombine(PAGE_KERNEL);
- pages = kmalloc_array(page_count, sizeof(struct page *), GFP_KERNEL);
+ pages = kmalloc_array(page_count, sizeof(*pages), GFP_KERNEL);
if (!pages)
return NULL;
@@ -520,7 +520,7 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
struct persistent_ram_zone *prz;
int ret = -ENOMEM;
- prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
+ prz = kzalloc(sizeof(*prz), GFP_KERNEL);
if (!prz)
goto err;
--
2.14.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] pstore: Improve a size determination in three functions
2017-08-16 19:22 ` [PATCH 2/3] pstore: Improve a size determination in three functions SF Markus Elfring
@ 2017-08-16 19:33 ` Kees Cook
2017-08-16 20:03 ` SF Markus Elfring
0 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2017-08-16 19:33 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Anton Vorontsov, Colin Cross, Tony Luck, kernel-janitors, LKML
On Wed, Aug 16, 2017 at 12:22 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 16 Aug 2017 20:50:15 +0200
>
> Replace the specification of data types by pointer dereferences
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
Agreed; this is a robustness change in that changes to the variable
structure will be correctly flowed through to the allocations.
>
> This issue was detected by using the Coccinelle software.
Which script detected this?
-Kees
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> fs/pstore/ram.c | 3 +--
> fs/pstore/ram_core.c | 4 ++--
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 42d27e5fac9f..0ef95c384bed 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -293,8 +293,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
> */
> struct persistent_ram_zone *tmp_prz, *prz_next;
>
> - tmp_prz = kzalloc(sizeof(struct persistent_ram_zone),
> - GFP_KERNEL);
> + tmp_prz = kzalloc(sizeof(*tmp_prz), GFP_KERNEL);
> if (!tmp_prz)
> return -ENOMEM;
> free_prz = true;
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index fafa8af1289c..5d9f7280d757 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -405,7 +405,7 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size,
> else
> prot = pgprot_writecombine(PAGE_KERNEL);
>
> - pages = kmalloc_array(page_count, sizeof(struct page *), GFP_KERNEL);
> + pages = kmalloc_array(page_count, sizeof(*pages), GFP_KERNEL);
> if (!pages)
> return NULL;
>
> @@ -520,7 +520,7 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size,
> struct persistent_ram_zone *prz;
> int ret = -ENOMEM;
>
> - prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
> + prz = kzalloc(sizeof(*prz), GFP_KERNEL);
> if (!prz)
> goto err;
>
> --
> 2.14.0
>
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: pstore: Improve a size determination in three functions
2017-08-16 19:33 ` Kees Cook
@ 2017-08-16 20:03 ` SF Markus Elfring
0 siblings, 0 replies; 9+ messages in thread
From: SF Markus Elfring @ 2017-08-16 20:03 UTC (permalink / raw)
To: Kees Cook; +Cc: Anton Vorontsov, Colin Cross, Tony Luck, kernel-janitors, LKML
> Which script detected this?
* checkpatch.pl
* The following approach for the semantic patch language.
@replacement@
identifier action, var, work;
statement s1, s2;
statement list sl;
type T, X;
@@
T work(...)
{
... when any
X* var;
... when any
var = action(...,
sizeof(
- X
+ *var
),
...
)
... when any
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] pstore: Adjust two checks for null pointers
2017-08-16 19:20 [PATCH 0/3] pstore: Adjustments for some function implementations SF Markus Elfring
2017-08-16 19:21 ` [PATCH 1/3] pstore: Delete six error messages for a failed memory allocation SF Markus Elfring
2017-08-16 19:22 ` [PATCH 2/3] pstore: Improve a size determination in three functions SF Markus Elfring
@ 2017-08-16 19:23 ` SF Markus Elfring
2017-08-16 19:32 ` Kees Cook
2 siblings, 1 reply; 9+ messages in thread
From: SF Markus Elfring @ 2017-08-16 19:23 UTC (permalink / raw)
To: Anton Vorontsov, Colin Cross, Kees Cook, Tony Luck,
kernel-janitors; +Cc: LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 16 Aug 2017 21:00:16 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The script “checkpatch.pl” pointed information out like the following.
Comparison to NULL could be written !…
Thus fix affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
fs/pstore/ram.c | 2 +-
fs/pstore/ram_core.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 0ef95c384bed..38b81868f7f9 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -333,7 +333,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
record->ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
record->buf = kmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
- if (record->buf = NULL) {
+ if (!record->buf) {
size = -ENOMEM;
goto out;
}
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 5d9f7280d757..e37b2d0cb9f4 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -223,7 +223,7 @@ static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
*/
prz->rs_decoder = init_rs(prz->ecc_info.symsize, prz->ecc_info.poly,
0, 1, prz->ecc_info.ecc_size);
- if (prz->rs_decoder = NULL) {
+ if (!prz->rs_decoder) {
pr_info("init_rs failed\n");
return -EINVAL;
}
--
2.14.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] pstore: Adjust two checks for null pointers
2017-08-16 19:23 ` [PATCH 3/3] pstore: Adjust two checks for null pointers SF Markus Elfring
@ 2017-08-16 19:32 ` Kees Cook
0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2017-08-16 19:32 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Anton Vorontsov, Colin Cross, Tony Luck, kernel-janitors, LKML
On Wed, Aug 16, 2017 at 12:23 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 16 Aug 2017 21:00:16 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The script “checkpatch.pl” pointed information out like the following.
>
> Comparison to NULL could be written !…
>
> Thus fix affected source code places.
I think these are find to do when changing other code, but as
stand-alone, I'm not interested in taking them, sorry!
-Kees
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> fs/pstore/ram.c | 2 +-
> fs/pstore/ram_core.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 0ef95c384bed..38b81868f7f9 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -333,7 +333,7 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record)
> record->ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
>
> record->buf = kmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
> - if (record->buf == NULL) {
> + if (!record->buf) {
> size = -ENOMEM;
> goto out;
> }
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index 5d9f7280d757..e37b2d0cb9f4 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -223,7 +223,7 @@ static int persistent_ram_init_ecc(struct persistent_ram_zone *prz,
> */
> prz->rs_decoder = init_rs(prz->ecc_info.symsize, prz->ecc_info.poly,
> 0, 1, prz->ecc_info.ecc_size);
> - if (prz->rs_decoder == NULL) {
> + if (!prz->rs_decoder) {
> pr_info("init_rs failed\n");
> return -EINVAL;
> }
> --
> 2.14.0
>
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 9+ messages in thread