Patchwork D1877: bdiff: Handle the possibility of an integer overflow when allocating

login
register
mail settings
Submitter phabricator
Date Jan. 17, 2018, 9:37 p.m.
Message ID <differential-rev-PHID-DREV-gvt7k57jmbk6pqger2wt-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26828/
State Superseded
Headers show

Comments

phabricator - Jan. 17, 2018, 9:37 p.m.
alex_gaynor created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

BRANCH
  alloc-overflow (bookmark) on default (branch)

REVISION DETAIL
  https://phab.mercurial-scm.org/D1877

AFFECTED FILES
  mercurial/bdiff.c

CHANGE DETAILS




To: alex_gaynor, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 17, 2018, 9:40 p.m.
durin42 accepted this revision.
durin42 added a comment.
This revision is now accepted and ready to land.


  Thanks!

REPOSITORY
  rHG Mercurial

BRANCH
  alloc-overflow (bookmark) on default (branch)

REVISION DETAIL
  https://phab.mercurial-scm.org/D1877

To: alex_gaynor, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
phabricator - Jan. 17, 2018, 9:42 p.m.
indygreg added inline comments.

INLINE COMMENTS

> bdiff.c:44
>  
> -	*lr = l = (struct bdiff_line *)malloc(sizeof(struct bdiff_line) * i);
> +	*lr = l = (struct bdiff_line *)calloc(i, sizeof(struct bdiff_line));
>  	if (!l)

Memory throughput is already a bottleneck in bdiff code. Is there an impact on `hg perfbdiff` with the change from `malloc()` to `calloc()`?

REPOSITORY
  rHG Mercurial

BRANCH
  alloc-overflow (bookmark) on default (branch)

REVISION DETAIL
  https://phab.mercurial-scm.org/D1877

To: alex_gaynor, #hg-reviewers, durin42
Cc: indygreg, durin42, mercurial-devel
phabricator - Jan. 17, 2018, 9:47 p.m.
durin42 added a comment.


  Before:
  
  ! wall 0.002096 comb 0.000000 user 0.000000 sys 0.000000 (best of 1329)
  
  After:
  
  ! wall 0.002094 comb 0.000000 user 0.000000 sys 0.000000 (best of 1355)
  
  so, uh, calloc is slightly faster? but not enough to even matter.

REPOSITORY
  rHG Mercurial

BRANCH
  alloc-overflow (bookmark) on default (branch)

REVISION DETAIL
  https://phab.mercurial-scm.org/D1877

To: alex_gaynor, #hg-reviewers, durin42
Cc: indygreg, durin42, mercurial-devel
phabricator - Jan. 17, 2018, 9:49 p.m.
durin42 added a comment.


  Wait, I'm a moron, I forgot to do ./hg instead of hg. New results:
  
  Before:
  
  ! wall 0.002058 comb 0.000000 user 0.000000 sys 0.000000 (best of 1293)
  
  After:
  
  ! wall 0.002107 comb 0.000000 user 0.000000 sys 0.000000 (best of 1342)
  
  so calloc() is very slightly slower, but it's so little I'm not inclined to sweat it.

REPOSITORY
  rHG Mercurial

BRANCH
  alloc-overflow (bookmark) on default (branch)

REVISION DETAIL
  https://phab.mercurial-scm.org/D1877

To: alex_gaynor, #hg-reviewers, durin42
Cc: indygreg, durin42, mercurial-devel

Patch

diff --git a/mercurial/bdiff.c b/mercurial/bdiff.c
--- a/mercurial/bdiff.c
+++ b/mercurial/bdiff.c
@@ -41,7 +41,7 @@ 
 	if (p == plast)
 		i++;
 
-	*lr = l = (struct bdiff_line *)malloc(sizeof(struct bdiff_line) * i);
+	*lr = l = (struct bdiff_line *)calloc(i, sizeof(struct bdiff_line));
 	if (!l)
 		return -1;