* [PATCH 1/2] staging/sep: Fix smatch false positive about potential NULL dereference in sep_main.c
@ 2013-02-19 12:07 Peter Huewe
2013-02-19 12:07 ` [PATCH 2/2] staging/sep: Check pointers before dereferencing (fix smatch warning) Peter Huewe
2013-02-19 12:25 ` [PATCH 1/2] staging/sep: Fix smatch false positive about potential NULL dereference in sep_main.c Dan Carpenter
0 siblings, 2 replies; 3+ messages in thread
From: Peter Huewe @ 2013-02-19 12:07 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Devendra Naga, Peter Huewe, Robert P. J. Day, Joe Perches, devel,
linux-kernel
Smatch complains about a potential NULL pointer dereference:
sep_main.c:2312 sep_construct_dma_tables_from_lli() error: potential
NULL dereference 'info_out_entry_ptr'.
info_out_entry_ptr is initialized with NULL and if info_in_entry_ptr is
not NULL it gets derefenced.
However info_out_entry_ptr is only NULL in the first iteration of the
while loop and in this case info_in_entry_ptr is also NULL (as indicated
by the comment /* If info entry is null - this is the first table built */
-> this is a false positive.
Nevertheless we add a check for info_out_entry_ptr to silence this
warning and make it more robust in regard to code changes.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/sep/sep_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 30e8d25..366d56b 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -2276,7 +2276,7 @@ static int sep_construct_dma_tables_from_lli(
table_data_size);
/* If info entry is null - this is the first table built */
- if (info_in_entry_ptr == NULL) {
+ if (info_in_entry_ptr == NULL || info_out_entry_ptr == NULL) {
/* Set the output parameters to physical addresses */
*lli_table_in_ptr =
sep_shared_area_virt_to_bus(sep, dma_in_lli_table_ptr);
--
1.7.8.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] staging/sep: Check pointers before dereferencing (fix smatch warning)
2013-02-19 12:07 [PATCH 1/2] staging/sep: Fix smatch false positive about potential NULL dereference in sep_main.c Peter Huewe
@ 2013-02-19 12:07 ` Peter Huewe
2013-02-19 12:25 ` [PATCH 1/2] staging/sep: Fix smatch false positive about potential NULL dereference in sep_main.c Dan Carpenter
1 sibling, 0 replies; 3+ messages in thread
From: Peter Huewe @ 2013-02-19 12:07 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Devendra Naga, Peter Huewe, Robert P. J. Day, Joe Perches, devel,
linux-kernel
smatch complains about two dereferenced before check issues:
sep_main.c:2898 sep_free_dma_tables_and_dcb() warn: variable dereferenced before check
'dma_ctx' (see line 2885)
sep_main.c:2898 sep_free_dma_tables_and_dcb() warn: variable dereferenced before check
'*dma_ctx' (see line 2885)
-> Move the checks to the top, but keep the semantics.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/sep/sep_main.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 366d56b..f5b7341 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -2880,6 +2880,8 @@ static int sep_free_dma_tables_and_dcb(struct sep_device *sep, bool isapplet,
dev_dbg(&sep->pdev->dev, "[PID%d] sep_free_dma_tables_and_dcb\n",
current->pid);
+ if (!dma_ctx || !*dma_ctx) /* nothing to be done here*/
+ return 0;
if (((*dma_ctx)->secure_dma == false) && (isapplet == true)) {
dev_dbg(&sep->pdev->dev, "[PID%d] handling applet\n",
@@ -2895,8 +2897,7 @@ static int sep_free_dma_tables_and_dcb(struct sep_device *sep, bool isapplet,
* Go over each DCB and see if
* tail pointer must be updated
*/
- for (i = 0; dma_ctx && *dma_ctx &&
- i < (*dma_ctx)->nr_dcb_creat; i++, dcb_table_ptr++) {
+ for (i = 0; i < (*dma_ctx)->nr_dcb_creat; i++, dcb_table_ptr++) {
if (dcb_table_ptr->out_vr_tail_pt) {
pt_hold = (unsigned long)dcb_table_ptr->
out_vr_tail_pt;
--
1.7.8.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] staging/sep: Fix smatch false positive about potential NULL dereference in sep_main.c
2013-02-19 12:07 [PATCH 1/2] staging/sep: Fix smatch false positive about potential NULL dereference in sep_main.c Peter Huewe
2013-02-19 12:07 ` [PATCH 2/2] staging/sep: Check pointers before dereferencing (fix smatch warning) Peter Huewe
@ 2013-02-19 12:25 ` Dan Carpenter
1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2013-02-19 12:25 UTC (permalink / raw)
To: Peter Huewe
Cc: Greg Kroah-Hartman, devel, linux-kernel, Joe Perches, Robert P. J. Day
On Tue, Feb 19, 2013 at 01:07:27PM +0100, Peter Huewe wrote:
> Smatch complains about a potential NULL pointer dereference:
>
> sep_main.c:2312 sep_construct_dma_tables_from_lli() error: potential
> NULL dereference 'info_out_entry_ptr'.
>
> info_out_entry_ptr is initialized with NULL and if info_in_entry_ptr is
> not NULL it gets derefenced.
> However info_out_entry_ptr is only NULL in the first iteration of the
> while loop and in this case info_in_entry_ptr is also NULL (as indicated
> by the comment /* If info entry is null - this is the first table built */
> -> this is a false positive.
>
> Nevertheless we add a check for info_out_entry_ptr to silence this
> warning and make it more robust in regard to code changes.
>
Smatch doesn't handle loops very well. Of course, all along I've
wanted to fix this, but it's a bit complicated so it could be
another year or two before it actually happens.
Generally, as a philosophy, I always say never to change the code
for false positives. It should be Smatch which changes.
Also the other thing is that with Smatch I deliberately allow more
false positives than GCC does. It's a trade off between being
ambitious in looking for bugs and being annoying to users.
When Smatch looks at this code it sees the else side as impossible
to reach. Perhaps I should add a hack in that if the code is in an
impossible to reach place then don't print a warning... It would
be better to just fix loop handling... I'm not sure.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-02-19 12:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-19 12:07 [PATCH 1/2] staging/sep: Fix smatch false positive about potential NULL dereference in sep_main.c Peter Huewe
2013-02-19 12:07 ` [PATCH 2/2] staging/sep: Check pointers before dereferencing (fix smatch warning) Peter Huewe
2013-02-19 12:25 ` [PATCH 1/2] staging/sep: Fix smatch false positive about potential NULL dereference in sep_main.c Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.