Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Coverity: accessing array before its start is dubious #13826

Closed
p5pRT opened this issue May 13, 2014 · 6 comments
Closed

[PATCH] Coverity: accessing array before its start is dubious #13826

p5pRT opened this issue May 13, 2014 · 6 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented May 13, 2014

Migrated from rt.perl.org#121857 (status was 'resolved')

Searchable as RT121857$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 13, 2014

From @jhi

S_do_delete_local did shady things with the Perl stack (&unsliced_keysv-1).

(patch by petermartini, detection by Coverity)

Patch attached.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 13, 2014

From @jhi

0001-Accessing-array-before-its-start-is-dubious.patch
From 0f5a78e4e88d3dc8c248cfeb603eab0a1f94f05d Mon Sep 17 00:00:00 2001
From: Jarkko Hietaniemi <jhi@iki.fi>
Date: Wed, 7 May 2014 12:04:37 -0400
Subject: [PATCH] Accessing array before its start is dubious.

Fix by petermartini.

Fix for Coverity perl5 CID 28909:
Out-of-bounds access (ARRAY_VS_SINGLETON)
ptr_arith: Using &unsliced_keysv as an array.
This might corrupt or misinterpret adjacent memory locations.
---
 pp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/pp.c b/pp.c
index 4ec6887..9949966 100644
--- a/pp.c
+++ b/pp.c
@@ -4533,15 +4533,15 @@ S_do_delete_local(pTHX)
     const MAGIC *mg;
     HV *stash;
     const bool sliced = !!(PL_op->op_private & OPpSLICE);
-    SV *unsliced_keysv = sliced ? NULL : POPs;
+    SV **unsliced_keysv = sliced ? NULL : sp--;
     SV * const osv = POPs;
-    SV **mark = sliced ? PL_stack_base + POPMARK : &unsliced_keysv-1;
+    SV **mark = sliced ? PL_stack_base + POPMARK : unsliced_keysv-1;
     dORIGMARK;
     const bool tied = SvRMAGICAL(osv)
 			    && mg_find((const SV *)osv, PERL_MAGIC_tied);
     const bool can_preserve = SvCANEXISTDELETE(osv);
     const U32 type = SvTYPE(osv);
-    SV ** const end = sliced ? SP : &unsliced_keysv;
+    SV ** const end = sliced ? SP : unsliced_keysv;
 
     if (type == SVt_PVHV) {			/* hash element */
 	    HV * const hv = MUTABLE_HV(osv);
@@ -4631,7 +4631,7 @@ S_do_delete_local(pTHX)
 	}
     }
     else if (gimme != G_VOID)
-	PUSHs(unsliced_keysv);
+	PUSHs(*unsliced_keysv);
 
     RETURN;
 }
-- 
1.9.2

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 15, 2014

From @tonycoz

On Tue May 13 08​:06​:15 2014, jhi wrote​:

S_do_delete_local did shady things with the Perl stack (&unsliced_keysv-1).

(patch by petermartini, detection by Coverity)

Patch attached.

Added to 5.21.1 blockers.

Tony

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 15, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 28, 2014

From @tsee

Thanks, applied as 626040f.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented May 28, 2014

@tsee - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this May 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant