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

SIGSEGV because mat array can overrun in opt_mergetable.c #3625

Closed
monetdb-team opened this issue Nov 30, 2020 · 0 comments
Closed

SIGSEGV because mat array can overrun in opt_mergetable.c #3625

monetdb-team opened this issue Nov 30, 2020 · 0 comments

Comments

@monetdb-team
Copy link

@monetdb-team monetdb-team commented Nov 30, 2020

Date: 2014-11-21 19:15:31 +0100
From: Richard Hughes <<richard.monetdb>>
To: MonetDB5 devs <>
Version: 11.19.3 (Oct2014)
CC: @njnes

Last updated: 2015-08-28 13:42:18 +0200

Comment 20453

Date: 2014-11-21 19:15:31 +0100
From: Richard Hughes <<richard.monetdb>>

Created attachment 310
proposed fix

I reproducibly crashed mserver5 using the statement "create table foo as select distinct * from bar with data;" where bar has 80 columns. The exact location of the crashes varied considerably, but they were always within OPTmergetableImplementation() or one of its children.

The problem was that the 'mat' array was being overrun and was randomly overwriting (usually) malloced InstrPtr instances. mat is initialized on line 1453 to mb->vtop elements, however there are numerous newTmpVariable() calls throughout that optimizer which can grow vtop considerably after mat has been created, and hence apparently grow the storage requirement for mat correspondingly.

It's possible that I did this to myself because the patch from bug #3548 can produce more complicated programs than would otherwise be the case, but I see no reason why a sufficiently arcane program couldn't be created even without that patch, and the statement in question only reads from one table so the multi-mitosis code shouldn't even be doing anything.

The attached patch simply mechanically changes mat_t to matlist_t {mat_t *v;int top;int size;}, i.e. a standard resizeable array implementation. This makes my test case work. You may be able to think of a smarter way to do it (or may completely disagree with my analysis).

Attached file: opt-mergetable-mat-overrun.patch (text/plain, 30112 bytes)
Description: proposed fix

Comment 20993

Date: 2015-07-11 11:45:08 +0200
From: @njnes

tooks some time, but applied patch

Comment 21209

Date: 2015-08-28 13:42:18 +0200
From: @sjoerdmullender

Jul2015 has been released.

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