You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by SuJinpei <gi...@git.apache.org> on 2018/01/18 12:32:58 UTC
[GitHub] trafodion pull request #1404: Trafodion 2916
GitHub user SuJinpei opened a pull request:
https://github.com/apache/trafodion/pull/1404
Trafodion 2916
**the root cause**: the allocating size of str is determined by max field length + 128(here is 1500+128), but during loading, str will be filled with actual field data exceeding this size(here maybe 2000 characters) if str overflow the client app will crash.
**solution**: increase the size of str to the size of one-time fread(default is 262144). there is still a risk of a crash when a user provides an incredibly large size of an invalid field.
**additional fix**: also fix other same type potential bugs in Oload2 and OloadJason.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/SuJinpei/incubator-trafodion trafodion-2916
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1404.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1404
----
commit dd231aaf95094400a59d4f99521d76a8771b625a
Author: SuJinpei <87...@...>
Date: 2018-01-18T11:43:46Z
fix TRAFODION-2916
commit 44a05e0793f27ec1b76e515c8441bd3605bbb090
Author: SuJinpei <87...@...>
Date: 2018-01-18T11:59:50Z
remove debug code
----
---
[GitHub] trafodion pull request #1404: Trafodion 2916
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1404#discussion_r162788763
--- Diff: core/conn/odb/src/odb.c ---
@@ -7080,7 +7080,7 @@ static void Oload(int eid)
mfl = (size_t) etab[eid].td[i].Osize;
/* Allocate field buffer */
- if ( (str = (char *)malloc (mfl + 128)) == (void *)NULL ) {
+ if ( (str = (char *)calloc (1, etab[eid].buffsz + 1)) == (void *)NULL ) {
--- End diff --
Is the buffer str used to read in source data for string column and reused for the next row/column? I am anxious that we do not do this large allocation repeatedly. I feel this is addressed already, the question is more of a sanity check as I cannot trace the variable str back far enough.
---
[GitHub] trafodion pull request #1404: Trafodion 2916
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1404
---
[GitHub] trafodion pull request #1404: Trafodion 2916
Posted by SuJinpei <gi...@git.apache.org>.
Github user SuJinpei commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1404#discussion_r162845411
--- Diff: core/conn/odb/src/odb.c ---
@@ -7080,7 +7080,7 @@ static void Oload(int eid)
mfl = (size_t) etab[eid].td[i].Osize;
/* Allocate field buffer */
- if ( (str = (char *)malloc (mfl + 128)) == (void *)NULL ) {
+ if ( (str = (char *)calloc (1, etab[eid].buffsz + 1)) == (void *)NULL ) {
--- End diff --
Hi Suresh
str will only be allocated one time for each loading thread.
---