tools: acrn-crashlog: Fix potential issues under acrnprobe

This patch is to fix potential issues, which are reported by static
analysis tool, for acrnprobe.

Changes:
1. Check the return value of sender_id() and get_sender_by_name(),
   since it could be -1 or NULL.
2. Remove the parameter len from create_event, take parameter path
   as a NULL-terminated string as default.
3. Modify for_each_* functions to avoid overflow.

Signed-off-by: Liu Xinwu <xinwu.liu@intel.com>
Reviewed-by: Zhi Jin <zhi.jin@intel.com>
Acked-by: Chen Gang <gang.c.chen@intel.com>
This commit is contained in:
xiaojin2 2018-06-08 13:15:29 +08:00 committed by lijinxia
parent 0c39b9cddc
commit 3e8e607d5b
7 changed files with 123 additions and 67 deletions

View File

@ -119,17 +119,22 @@ static int refresh_key_synced_stage1(struct sender_t *sender, struct vm_t *vm,
char *key, enum refresh_type_t type)
{
char log_new[64];
char *log_vmrecordid = sender->log_vmrecordid;
char *log_vmrecordid;
int sid;
if (!key || !sender || !vm)
return -1;
sid = sender_id(sender);
if (sid == -1)
return -1;
log_vmrecordid = sender->log_vmrecordid;
/* the length of key must be 20, and its value can not be
* 00000000000000000000.
*/
if ((strlen(key) == 20) &&
strcmp(key, "00000000000000000000")) {
sprintf(vm->last_synced_line_key[sender_id(sender)],
sprintf(vm->last_synced_line_key[sid],
"%s", key);
if (type == MM_ONLY)
return 0;
@ -158,6 +163,9 @@ static int refresh_key_synced_stage2(struct mm_file_t *m_vm_records, char *key)
if (*key) {
begin = strstr(m_vm_records->begin, " <==");
end = strrstr(m_vm_records->begin, key);
if (!begin || !end)
return -1;
end = strchr(end, '\n');
for (p = begin; p < end; p++) {
@ -183,6 +191,8 @@ static int get_vm_history(struct vm_t *vm, struct sender_t *sender,
return -1;
sid = sender_id(sender);
if (sid == -1)
return -1;
snprintf(vm_history, sizeof(vm_history), "/tmp/%s_%s",
"vm_hist", vm->name);
@ -221,6 +231,9 @@ static void sync_lines_stage1(struct sender_t *sender, void *data[])
char *vm_format = "%*[^ ]%*[ ]%[^ ]%*c";
sid = sender_id(sender);
if (sid == -1)
return;
for_each_vm(id, vm, conf) {
if (!vm)
continue;
@ -287,6 +300,10 @@ static void sync_lines_stage2(struct sender_t *sender, void *data[],
strerror(errno));
return;
}
if (!m_vm_records->size) {
LOGE("size(0b) of (%s)\n", sender->log_vmrecordid);
return;
}
cursor = strstr(m_vm_records->begin, " <==");
if (!cursor)
@ -334,11 +351,19 @@ out:
static void get_last_line_synced(struct sender_t *sender)
{
int id;
int sid;
int ret;
struct vm_t *vm;
char vmkey[SHA_DIGEST_LENGTH + 10] = {0};
char word[256];
if (!sender)
return;
sid = sender_id(sender);
if (sid == -1)
return;
for_each_vm(id, vm, conf) {
if (!vm)
continue;
@ -347,7 +372,7 @@ static void get_last_line_synced(struct sender_t *sender)
continue;
/* generally only exec for each vm once */
if (vm->last_synced_line_key[sender_id(sender)][0])
if (vm->last_synced_line_key[sid][0])
continue;
snprintf(word, sizeof(word), "%s ", vm->name);
@ -406,6 +431,7 @@ static char *setup_loop_dev(void)
* launch_UOS.sh, we need mount its data partition to loop device
*/
char *out;
char *end;
char loop_dev_tmp[32];
int i;
@ -440,7 +466,9 @@ static char *setup_loop_dev(void)
}
}
}
*strchr(loop_dev, '\n') = 0;
end = strchr(loop_dev, '\n');
if (end)
*end = 0;
out = exec_out2mem("fdisk -lu %s", android_img);
if (!out) {

View File

@ -41,9 +41,9 @@ static struct channel_t channels[] = {
};
#define for_each_channel(i, channel) \
for (i = 0, channel = &channels[0]; \
i < (int)ARRAY_SIZE(channels); \
i++, channel++)
for (i = 0; \
(i < (int)ARRAY_SIZE(channels)) && (channel = &channels[i]); \
i++)
/**
* Helper function to create a event and fill event_t structure.
@ -53,30 +53,35 @@ static struct channel_t channels[] = {
* @param private The corresponding configuration info to the event.
* @param watchfd For watch channel, so far, only used by inotify.
* @param path File which trigger this event.
* @param len Length of path.
*
* @return a pointer to the filled event if successful,
* or NULL on error.
*/
static struct event_t *create_event(enum event_type_t event_type,
char *channel, void *private,
int watchfd, char *path, int len)
int watchfd, char *path)
{
struct event_t *e;
int path_len = 0;
if (len < 0)
return NULL;
if (path) {
path_len = strlen(path);
if (path_len > PATH_MAX) {
LOGE("invalid path, drop event.\n");
return NULL;
}
}
e = malloc(sizeof(*e) + len + 1);
e = malloc(sizeof(*e) + path_len + 1);
if (e) {
memset(e, 0, sizeof(*e) + len + 1);
memset(e, 0, sizeof(*e) + path_len + 1);
e->watchfd = watchfd;
e->channel = channel;
e->private = (void *)private;
e->private = private;
e->event_type = event_type;
if (path && (len > 0)) {
e->len = len;
strncpy(e->path, path, len);
if (path_len > 0) {
e->len = path_len;
memcpy(e->path, path, path_len + 1);
}
} else {
LOGE("malloc failed, error (%s)\n", strerror(errno));
@ -100,7 +105,7 @@ static void channel_oneshot(struct channel_t *cnl)
LOGD("initializing channel %s ...\n", cname);
e = create_event(REBOOT, cname, NULL, 0, NULL, 0);
e = create_event(REBOOT, cname, NULL, 0, NULL);
if (e)
event_enqueue(e);
@ -116,8 +121,7 @@ static void channel_oneshot(struct channel_t *cnl)
!strcmp("file", crash->trigger->type) &&
file_exists(crash->trigger->path)) {
e = create_event(CRASH, cname, (void *)crash,
0, crash->trigger->path,
strlen(crash->trigger->path));
0, crash->trigger->path);
if (e)
event_enqueue(e);
}
@ -134,7 +138,7 @@ static void channel_oneshot(struct channel_t *cnl)
!strcmp("file", info->trigger->type) &&
file_exists(info->trigger->path)) {
e = create_event(INFO, cname, (void *)info,
0, NULL, 0);
0, NULL);
if (e)
event_enqueue(e);
}
@ -156,7 +160,7 @@ static struct polling_job_t {
static void polling_vm(union sigval v __attribute__((unused)))
{
struct event_t *e = create_event(VM, "polling", NULL, 0, NULL, 0);
struct event_t *e = create_event(VM, "polling", NULL, 0, NULL);
if (e)
event_enqueue(e);
@ -319,6 +323,7 @@ static int receive_inotify_events(struct channel_t *channel)
int read_left;
char buf[256];
char *p;
char *name;
struct event_t *e;
struct inotify_event *ievent;
enum event_type_t event_type;
@ -360,9 +365,14 @@ static int receive_inotify_events(struct channel_t *channel)
if (event_type == UNKNOWN) {
LOGE("get a unknown event\n");
} else {
if (!ievent->len)
name = NULL;
else
name = ievent->name;
e = create_event(event_type, channel->name,
private, channel->fd,
ievent->name, ievent->len);
name);
if (e)
event_enqueue(e);
}
@ -384,7 +394,7 @@ static int receive_inotify_events(struct channel_t *channel)
*/
static void heart_beat(void)
{
struct event_t *e = create_event(HEART_BEAT, NULL, NULL, 0, NULL, 0);
struct event_t *e = create_event(HEART_BEAT, NULL, NULL, 0, NULL);
if (e)
event_enqueue(e);

View File

@ -233,7 +233,7 @@ static struct crash_t *crash_reclassify_by_content(struct crash_t *rcrash,
return NULL;
if (trfile) {
ret = read_full_binary_file(trfile, &size, &file);
ret = read_file(trfile, &size, &file);
if (ret == -1) {
LOGE("read %s failed, error (%s)\n",
trfile, strerror(errno));

View File

@ -132,6 +132,9 @@ void hist_raise_event(char *event, char *type, char *log, char *lastuptime,
/* here means user have configured the crashlog sender */
crashlog = get_sender_by_name("crashlog");
if (!crashlog)
return;
maxlines = atoi(crashlog->maxlines);
if (++current_lines >= maxlines) {
LOGW("lines of (%s) meet quota %d, backup... Pls clean!\n",
@ -139,9 +142,11 @@ void hist_raise_event(char *event, char *type, char *log, char *lastuptime,
backup_history();
}
get_current_time_long(eventtime);
entry.eventtime = eventtime;
ret = get_current_time_long(eventtime);
if (ret <= 0)
return;
entry.eventtime = eventtime;
entry_to_history_line(&entry, line);
ret = append_file(history_file, line);
if (ret < 0) {
@ -165,6 +170,9 @@ void hist_raise_uptime(char *lastuptime)
/* user have configured the crashlog sender */
crashlog = get_sender_by_name("crashlog");
if (!crashlog)
return;
uptime = crashlog->uptime;
uptime_hours = atoi(uptime->eventhours);

View File

@ -111,54 +111,54 @@ struct conf_t {
struct conf_t conf;
#define for_each_sender(id, sender, conf) \
for (id = 0, sender = conf.sender[0]; \
id < SENDER_MAX; \
id++, sender = conf.sender[id])
for (id = 0; \
id < SENDER_MAX && (sender = conf.sender[id]); \
id++)
#define for_each_trigger(id, trigger, conf) \
for (id = 0, trigger = conf.trigger[0]; \
id < TRIGGER_MAX; \
id++, trigger = conf.trigger[id])
for (id = 0; \
id < TRIGGER_MAX && (trigger = conf.trigger[id]); \
id++)
#define for_each_vm(id, vm, conf) \
for (id = 0, vm = conf.vm[0]; \
id < VM_MAX; \
id++, vm = conf.vm[id])
for (id = 0; \
id < VM_MAX && (vm = conf.vm[id]); \
id++)
#define for_each_syncevent_vm(id, event, vm) \
for (id = 0, event = vm->syncevent[0]; \
id < VM_EVENT_TYPE_MAX; \
id++, event = vm->syncevent[id])
for (id = 0; \
id < VM_EVENT_TYPE_MAX && (event = vm->syncevent[id]); \
id++)
#define for_each_info(id, info, conf) \
for (id = 0, info = conf.info[0]; \
id < INFO_MAX; \
id++, info = conf.info[id])
for (id = 0; \
id < INFO_MAX && (info = conf.info[id]); \
id++)
#define for_each_log(id, log, conf) \
for (id = 0, log = conf.log[0]; \
id < LOG_MAX; \
id++, log = conf.log[id])
for (id = 0; \
id < LOG_MAX && (log = conf.log[id]); \
id++)
#define for_each_crash(id, crash, conf) \
for (id = 0, crash = conf.crash[0]; \
id < CRASH_MAX; \
id++, crash = conf.crash[id])
for (id = 0; \
id < CRASH_MAX && (crash = conf.crash[id]); \
id++)
#define for_each_log_collect(id, log, type) \
for (id = 0, log = type->log[0]; \
id < LOG_MAX; \
id++, log = type->log[id])
for (id = 0; \
id < LOG_MAX && (log = type->log[id]); \
id++)
#define for_each_content_crash(id, content, crash) \
for (id = 0, content = crash->content[0]; \
id < CONTENT_MAX; \
id++, content = crash->content[id])
for (id = 0; \
id < CONTENT_MAX && (content = crash->content[id]); \
id++)
#define for_each_content_expression(id, content, exp) \
for (id = 0, content = exp[0]; \
id < CONTENT_MAX; \
id++, content = exp[id])
for (id = 0; \
id < CONTENT_MAX && (content = exp[id]); \
id++)
#define exp_valid(exp) \
(__extension__ \
@ -175,9 +175,9 @@ struct conf_t conf;
)
#define for_each_expression_crash(id, exp, crash) \
for (id = 0, exp = crash->mightcontent[0]; \
id < EXPRESSION_MAX; \
id++, exp = crash->mightcontent[id])
for (id = 0; \
id < EXPRESSION_MAX && (exp = crash->mightcontent[id]); \
id++)
#define for_crash_children(crash, tcrash) \
TAILQ_FOREACH(crash, &tcrash->children, entries)

View File

@ -79,10 +79,10 @@ int get_current_time_long(char buf[32])
time(&t);
time_val = localtime((const time_t *)&t);
if (!time_val)
return -1;
strftime(buf, 32, "%Y-%m-%d/%H:%M:%S ", time_val);
return 0;
return strftime(buf, 32, "%Y-%m-%d/%H:%M:%S ", time_val);
}
/**
@ -286,6 +286,9 @@ static int reserve_log_folder(enum e_dir_mode mode, char *dir,
unsigned int maxdirs;
crashlog = get_sender_by_name("crashlog");
if (!crashlog)
return -1;
outdir = crashlog->outdir;
switch (mode) {
@ -352,7 +355,9 @@ void generate_crashfile(char *dir, char *event, char *hashkey,
const int fmtsize = 128;
int filesize;
get_current_time_long(datetime);
ret = get_current_time_long(datetime);
if (ret <= 0)
return;
get_uptime_string(uptime, &hours);
filesize = fmtsize + strlen(event) +

View File

@ -185,7 +185,7 @@ static void get_log_rotation(struct log_t *log, char *desdir)
if (count > 2) {
for (i = 0; i < count; i++) {
name = strrchr(files[i], '/') + 1;
if (!strstr(name, prefix))
if (!name && !strstr(name, prefix))
continue;
number = atoi(strrchr(name, '.') + 1);
@ -793,7 +793,7 @@ static void crashlog_send_crash(struct event_t *e)
char *data1;
char *data2;
int id;
int ret;
int ret = 0;
int quota;
struct crash_t *rcrash = (struct crash_t *)e->private;
@ -931,8 +931,13 @@ static void crashlog_send_reboot(void)
{
char reason[MAXLINESIZE];
char *key;
struct sender_t *crashlog;
if (swupdated(get_sender_by_name("crashlog"))) {
crashlog = get_sender_by_name("crashlog");
if (!crashlog)
return;
if (swupdated(crashlog)) {
key = generate_event_id("INFO", "SWUPDATE");
if (key == NULL) {
LOGE("generate event id failed, error (%s)\n",