I agree. And Ack, Mathi. > -----Original Message----- > From: Lennart Lund [mailto:[email protected]] > Sent: Thursday, October 15, 2015 12:48 PM > To: Vu Nguyen M; Mathivanan Naickan Palanivelu; Giang Do T > Cc: [email protected] > Subject: RE: [PATCH 1 of 1] log: incorrect verification the string length of > saLogStreamFileName value [#1493] > > Hi Vu, > > I think that since #1493 is a crash that can be introduced using immcfg it > should probably remain a defect and #279 is definitely an enhancement. This > means that my suggestion to create a fix that solves both is not possible so > they have to be fixed separately even if fixing #279 probably will fix #1493 > as > well. > > Maybe Mathi wants to comment this as well? > > Thanks > Lennart > > > -----Original Message----- > > From: Vu Minh Nguyen [mailto:[email protected]] > > Sent: den 14 oktober 2015 07:30 > > To: Lennart Lund; [email protected]; Giang Do T > > Cc: [email protected] > > Subject: RE: [PATCH 1 of 1] log: incorrect verification the string > > length of saLogStreamFileName value [#1493] > > > > Hi Lennart, > > > > This ticket type is defect. #279 is an enhancement. > > If fixing #279 in this ticket, I would like to propose changing #1493 > > type to enhancement. > > > > Regards, > > Vu > > > > > > >-----Original Message----- > > >From: Lennart Lund [mailto:[email protected]] > > >Sent: Tuesday, October 13, 2015 6:15 PM > > >To: Vu Nguyen M; [email protected]; Giang Do T > > >Cc: [email protected] > > >Subject: RE: [PATCH 1 of 1] log: incorrect verification the string > > >length > > of > > >saLogStreamFileName value [#1493] > > > > > >Hi Vu, > > > > > >Yes it can be applied on top of the 1466 patch. > > > > > >Comments: > > >A check of lie name length is also done in the > > >dec_lstr_open_sync_msg() function in lgs_mds.c maybe you should look > at this as well. > > > > > >Maybe you should consider fixing #279 together with this one. > > > > > >Thanks > > >Lennart > > > > > >> -----Original Message----- > > >> From: Vu Minh Nguyen [mailto:[email protected]] > > >> Sent: den 13 oktober 2015 10:51 > > >> To: Lennart Lund; [email protected]; Giang Do T > > >> Cc: [email protected] > > >> Subject: RE: [PATCH 1 of 1] log: incorrect verification the string > > >> length > > of > > >> saLogStreamFileName value [#1493] > > >> > > >> Hi Lennart, > > >> > > >> Could you please try to apply the patch on top of #1466 one? Thanks. > > >> > > >> Regards, > > >> Vu > > >> > > >> > > >> >-----Original Message----- > > >> >From: Lennart Lund [mailto:[email protected]] > > >> >Sent: Tuesday, October 13, 2015 3:15 PM > > >> >To: Vu Nguyen M; [email protected]; Giang Do T > > >> >Cc: [email protected] > > >> >Subject: RE: [PATCH 1 of 1] log: incorrect verification the string > > length > > >> of > > >> >saLogStreamFileName value [#1493] > > >> > > > >> >Hi Vu, > > >> > > > >> >The patch cannot be applied. Something wrong when merging test > > >> >code > > >> > > > >> >Thanks > > >> >Lennart > > >> > > > >> >> -----Original Message----- > > >> >> From: Vu Minh Nguyen [mailto:[email protected]] > > >> >> Sent: den 9 oktober 2015 10:32 > > >> >> To: [email protected]; Lennart Lund; Giang Do T > > >> >> Cc: [email protected] > > >> >> Subject: [PATCH 1 of 1] log: incorrect verification the string > > >> >> length > > of > > >> >> saLogStreamFileName value [#1493] > > >> >> > > >> >> osaf/libs/agents/saf/lga/lga_api.c | 2 +- > > >> >> osaf/libs/common/logsv/include/lgsv_defs.h | 10 +++++ > > >> >> osaf/services/saf/logsv/lgs/lgs_imm.c | 22 +++++++++++- > > >> >> tests/logsv/tet_LogOiOps.c | 47 > > >> +++++++++++++++++++++++++- > > >> >> tests/logsv/tet_saLogStreamOpen_2.c | 54 > > >> >> ++++++++++++++++++++++++++++++ > > >> >> 5 files changed, 131 insertions(+), 4 deletions(-) > > >> >> > > >> >> > > >> >> Correct the verification rule for the filename length. > > >> >> > > >> >> diff --git a/osaf/libs/agents/saf/lga/lga_api.c > > >> >> b/osaf/libs/agents/saf/lga/lga_api.c > > >> >> --- a/osaf/libs/agents/saf/lga/lga_api.c > > >> >> +++ b/osaf/libs/agents/saf/lga/lga_api.c > > >> >> @@ -561,7 +561,7 @@ static SaAisErrorT validate_open_params( > > >> >> /* Check implementation specific string length */ > > >> >> if (NULL != logFileCreateAttributes) { > > >> >> len = strlen(logFileCreateAttributes- > > >> >> >logFileName); > > >> >> - if ((len == 0) || (len > NAME_MAX)) { > > >> >> + if ((len == 0) || (len > LOG_NAME_MAX)) { > > >> >> TRACE("logFileName"); > > >> >> return > > >> >> SA_AIS_ERR_INVALID_PARAM; > > >> >> } > > >> >> diff --git a/osaf/libs/common/logsv/include/lgsv_defs.h > > >> >> b/osaf/libs/common/logsv/include/lgsv_defs.h > > >> >> --- a/osaf/libs/common/logsv/include/lgsv_defs.h > > >> >> +++ b/osaf/libs/common/logsv/include/lgsv_defs.h > > >> >> @@ -25,4 +25,14 @@ > > >> >> // Waiting time in library for sync send, unit 10ms #define > > >> >> LGS_WAIT_TIME 1000 > > >> >> > > >> >> +// The length of '_yyyymmdd_hhmmss_yyyymmdd_hhmmss.log' > > >> including > > >> >> null-termination char > > >> >> +#define LOG_TAIL_MAX 37 > > >> >> + > > >> >> +/** The log file name format: > > >> <filename>_<createtime>_<closetime>.log > > >> >> + * '_<createtime>_<closetime>.log' part is appended by logsv > > >> >> + and has > > >> >> above format, > > >> >> + * and '<filename>' part can set by user. Therefore, limit the > > >> >> + length > > of > > >> >> <filename> from user, > > >> >> + * to make sure the log file name does not exceed the maximum > > length > > >> >> (NAME_MAX). > > >> >> + */ > > >> >> +#define LOG_NAME_MAX (NAME_MAX - LOG_TAIL_MAX) > > >> >> + > > >> >> #endif /* LGSV_DEFS_H */ > > >> >> diff --git a/osaf/services/saf/logsv/lgs/lgs_imm.c > > >> >> b/osaf/services/saf/logsv/lgs/lgs_imm.c > > >> >> --- a/osaf/services/saf/logsv/lgs/lgs_imm.c > > >> >> +++ b/osaf/services/saf/logsv/lgs/lgs_imm.c > > >> >> @@ -1429,7 +1429,16 @@ static SaAisErrorT check_attr_validity(S > > >> >> TRACE("NULL > > >> >> pointer to saLogStreamPathName"); > > >> >> goto done; > > >> >> } > > >> >> - > > >> >> + > > >> >> + if (strlen(i_pathName) > > > >> >> PATH_MAX) { > > >> >> + /* Path name is too > > >> >> long */ > > >> >> + rc = > > >> >> SA_AIS_ERR_BAD_OPERATION; > > >> >> + > > >> >> report_oi_error(immOiHandle, opdata->ccbId, > > >> >> + > > >> >> "saLogStreamPathName is so long"); > > >> >> + > > >> >> TRACE("saLogStreamPathName is so long (max: %d)", > PATH_MAX); > > >> >> + goto done; > > >> >> + } > > >> >> + > > >> >> if > > >> >> (lgs_relative_path_check_ts(i_pathName) == true) { > > >> >> > > >> >> report_oi_error(immOiHandle, opdata->ccbId, > > >> >> > > >> >> "Path %s not valid", i_pathName); @@ -1453,7 > +1462,16 @@ > > >> >> static SaAisErrorT check_attr_validity(S > > >> >> TRACE("NULL > > >> >> pointer to saLogStreamFileName"); > > >> >> goto done; > > >> >> } > > >> >> - > > >> >> + > > >> >> + if (strlen(i_fileName) > > > >> >> LOG_NAME_MAX) { > > >> >> + /* File name is too > > >> >> long */ > > >> >> + rc = > > >> >> SA_AIS_ERR_BAD_OPERATION; > > >> >> + > > >> >> report_oi_error(immOiHandle, opdata->ccbId, > > >> >> + > > >> >> "saLogStreamFileName is so long"); > > >> >> + > > >> >> TRACE("saLogStreamFileName is so long (max: %d)", > > >> >> LOG_NAME_MAX); > > >> >> + goto done; > > >> >> + } > > >> >> + > > >> >> if > > >> >> (chk_filepath_stream_exist(i_fileName, i_pathName, > > >> >> > > >> >> stream, opdata->operationType)) { > > >> >> > > >> >> report_oi_error(immOiHandle, opdata->ccbId, diff --git > > >> >> a/tests/logsv/tet_LogOiOps.c b/tests/logsv/tet_LogOiOps.c > > >> >> --- a/tests/logsv/tet_LogOiOps.c > > >> >> +++ b/tests/logsv/tet_LogOiOps.c > > >> >> @@ -3318,6 +3318,50 @@ void verFixLogRec_Min(void) > > >> >> rc_validate(WEXITSTATUS(rc), 1); } > > >> >> > > >> >> + > > >> >> +/** > > >> >> + * Test cases to verify #1493 > > >> >> + */ > > >> >> + > > >> >> +/* Verify that it is not allowed to pass a string value to > > >> >> + * saLogStreamFileName which its length is larger than 218. > > >> >> + */ > > >> >> +void verFilenameLength_01(void) { > > >> >> + int rc; > > >> >> + char command[500]; > > >> >> + char fileName[220]; > > >> >> + > > >> >> + /* Create a string with 218 characters */ > > >> >> + memset(fileName, 'A', 218); > > >> >> + fileName[219] = '\0'; > > >> >> + > > >> >> + /* Create an configurable obj class with invalid filename > > >> >> +length > > >> >> */ > > >> >> + sprintf(command, "immcfg -c SaLogStreamConfig > > >> safLgStrCfg=TestLength > > >> >> " > > >> >> + "-a saLogStreamFileName=\"%s\" -a > > >> >> saLogStreamPathName=. " > > >> >> + "2> /dev/null", fileName); > > >> >> + rc = system(command); > > >> >> + rc_validate(WEXITSTATUS(rc), 1); } > > >> >> + > > >> >> +/* Modify an existing value */ > > >> >> +void verFilenameLength_02(void) { > > >> >> + int rc; > > >> >> + char command[500]; > > >> >> + char fileName[220]; > > >> >> + > > >> >> + /* Create a string with 218 characters */ > > >> >> + memset(fileName, 'A', 218); > > >> >> + fileName[219] = '\0'; > > >> >> + > > >> >> + /* Create an configurable obj class with invalid filename > > >> >> +length > > >> >> */ > > >> >> + sprintf(command, "immcfg -a saLogStreamFileName=. -a > > >> >> saLogStreamPathName=\"%s\" %s " > > >> >> + "2> /dev/null", fileName, > > >> >> SA_LOG_STREAM_ALARM); > > >> >> + rc = system(command); > > >> >> + rc_validate(WEXITSTATUS(rc), 1); } > > >> >> + > > >> >> __attribute__ ((constructor)) static void > > >> saOiOperations_constructor(void) > > >> >> { > > >> >> /* Stream objects */ > > >> >> @@ -3419,7 +3463,6 @@ void verFixLogRec_Min(void) > > >> >> test_case_add(5, verCCBWithValidValues, "CCB Object > Modify > > >> >> many attributes with valid values, OK"); > > >> >> test_case_add(5, verCCBWithInvalidValues, "CCB Object > Modify > > >> >> many attributes with one invalid values, ERR"); > > >> >> > > >> >> - > > >> >> /* Stream configuration object */ > > >> >> /* Tests for create */ > > >> >> test_suite_add(6, "LOG OI tests, Stream configuration object > > >> >> attribute validation"); @@ -3437,6 +3480,7 @@ void > > >> >> verFixLogRec_Min(void) > > >> >> test_case_add(6, saLogOi_76, "Create: > > >> >> saLogStreamMaxFilesRotated < 128, Ok"); > > >> >> test_case_add(6, saLogOi_77, "Create: > > >> >> saLogStreamMaxFilesRotated > 128, ERR"); > > >> >> test_case_add(6, saLogOi_78, "Create: > > >> >> saLogStreamMaxFilesRotated == 128, ERR"); > > >> >> + test_case_add(6, verFilenameLength_01, "Create: > > >> >> saLogStreamFileName > 218 characters, ERR"); > > >> >> test_case_add(6, verMaxFilesRotated, "Create: > > >> >> saLogStreamMaxFilesRotated = 0, ERR"); > > >> >> test_case_add(6, verAdminOpOnConfClass, "Perform admin > op on > > >> >> configurable obj class, ERR"); > > >> >> > > >> >> @@ -3461,6 +3505,7 @@ void verFixLogRec_Min(void) > > >> >> test_case_add(6, saLogOi_113, "Modify: > > >> >> saLogStreamMaxFilesRotated < 128, Ok"); > > >> >> test_case_add(6, saLogOi_114, "Modify: > > >> >> saLogStreamMaxFilesRotated > 128, ERR"); > > >> >> test_case_add(6, saLogOi_115, "Modify: > > >> >> saLogStreamMaxFilesRotated == 128, ERR"); > > >> >> + test_case_add(6, verFilenameLength_02, "Modify: > > >> >> saLogStreamFileName > 218 characters, ERR"); > > >> >> > > >> >> /* Add test cases to test #1288 */ > > >> >> test_case_add(6, verMaxLogRecord_01, "Modify: > > >> >> saLogStreamFixedLogRecordSize == 0, write a record = 65535 > > >> >> bytes, > > OK"); > > >> >> diff --git a/tests/logsv/tet_saLogStreamOpen_2.c > > >> >> b/tests/logsv/tet_saLogStreamOpen_2.c > > >> >> --- a/tests/logsv/tet_saLogStreamOpen_2.c > > >> >> +++ b/tests/logsv/tet_saLogStreamOpen_2.c > > >> >> @@ -531,6 +531,58 @@ done: > > >> >> } > > >> >> } > > >> >> > > >> >> +/** > > >> >> + * Verify that logFileName length > 218 characters is not allowed. > > >> >> + */ > > >> >> +void saLogStreamOpen_2_48(void) { > > >> >> + SaAisErrorT rc = SA_AIS_OK; > > >> >> + SaNameT logStreamName = {.length = 0 }; > > >> >> + char fileName[220]; > > >> >> + > > >> >> + memset(fileName, 'A', 218); > > >> >> + fileName[219] = '\0'; > > >> >> + > > >> >> + strcpy((char *)logStreamName.value, > > >> SA_LOG_STREAM_APPLICATION1); > > >> >> + logStreamName.length = strlen((char *)logStreamName.value); > > >> >> + > > >> >> + SaLogFileCreateAttributesT_2 appLogFileCreateAttributes; > > >> >> + > > >> >> + /* App stream information */ > > >> >> + appLogFileCreateAttributes.logFilePathName = "appstream"; > > >> >> + appLogFileCreateAttributes.logFileName = fileName; > > >> >> + appLogFileCreateAttributes.maxLogFileSize = > > >> >> DEFAULT_APP_LOG_FILE_SIZE; > > >> >> + appLogFileCreateAttributes.maxLogRecordSize = > > >> >> DEFAULT_APP_LOG_REC_SIZE; > > >> >> + appLogFileCreateAttributes.haProperty = SA_TRUE; > > >> >> + appLogFileCreateAttributes.logFileFullAction = > > >> >> SA_LOG_FILE_FULL_ACTION_ROTATE; > > >> >> + appLogFileCreateAttributes.maxFilesRotated = 128; > > >> >> + appLogFileCreateAttributes.logFileFmt = NULL; > > >> >> + > > >> >> + rc = saLogInitialize(&logHandle, &logCallbacks, &logVersion); > > >> >> + if (rc != SA_AIS_OK) { > > >> >> + fprintf(stderr, "Failed at saLogInitialize: %d\n ", (int)rc); > > >> >> + test_validate(rc, SA_AIS_OK); > > >> >> + return; > > >> >> + } > > >> >> + > > >> >> + rc = saLogSelectionObjectGet(logHandle, &selectionObject); > > >> >> + if (rc != SA_AIS_OK) { > > >> >> + fprintf(stderr, "Failed at saLogSelectionObjectGet: %d > > >> >> + \n ", > > >> (int)rc); > > >> >> + test_validate(rc, SA_AIS_OK); > > >> >> + goto done; > > >> >> + } > > >> >> + > > >> >> + rc = saLogStreamOpen_2(logHandle, &logStreamName, > > >> >> &appLogFileCreateAttributes, > > >> >> + SA_LOG_STREAM_CREATE, > > >> >> + SA_TIME_ONE_SECOND, > > >> >> &logStreamHandle); > > >> >> + test_validate(rc, SA_AIS_ERR_INVALID_PARAM); > > >> >> + > > >> >> +done: > > >> >> + rc = saLogFinalize(logHandle); > > >> >> + if (rc != SA_AIS_OK) { > > >> >> + fprintf(stderr, "Failed to call salogFinalize: %d\n", > > >> >> +(int) > > rc); > > >> >> + } > > >> >> +} > > >> >> + > > >> >> extern void saLogStreamOpenAsync_2_01(void); extern void > > >> >> saLogStreamOpenCallbackT_01(void); > > >> >> extern void saLogWriteLog_01(void); @@ -607,6 +659,8 @@ extern > > >> >> void saLogStreamClose_01(void); > > >> >> test_case_add(2, saLogStreamClose_01, "saLogStreamClose OK"); > > >> >> test_case_add(2, saLogStreamOpen_2_46, "saLogStreamOpen_2 > > with > > >> >> maxFilesRotated = 0, ERR"); > > >> >> test_case_add(2, saLogStreamOpen_2_47, "saLogStreamOpen_2 > > with > > >> >> maxFilesRotated = 128, ERR"); > > >> >> + test_case_add(2, saLogStreamOpen_2_48, "saLogStreamOpen_2 > > with > > >> >> logFileName > 218 characters, ERR"); > > >> >> + > > >> >> test_case_add(2, verFixLogRec_Max_Err, "saLogStreamOpen_2 > > with > > >> >> maxLogRecordSize > MAX_RECSIZE, ERR"); > > >> >> test_case_add(2, verFixLogRec_Min_Err, "saLogStreamOpen_2 > > >> >> with maxLogRecordSize < 150, ERR"); } >
------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
