[RHEL6 LIBREPORT PATCH] Fix bugs discoverent by Coverity. rhbz#905051
Denys Vlasenko
dvlasenk at redhat.com
Wed Jun 12 11:11:52 UTC 2013
This is a partial backport of 93c6e6f9298928dcb140797d9fe8ff9038790423.
Most bugs are either outright bogus (rhbz_array_size is never <0)
or won't happen in practice (fstat won't fail on a file we just created
ourselves). Nevertheless, one bug is real:
if (flags & LIBREPORT_NOWAIT)...
is WRONG, because LIBREPORT_NOWAIT constant is in fact 0!
We must check for that bit this way:
if (!(flags & LIBREPORT_WAIT))...
Signed-off-by: Denys Vlasenko <dvlasenk at redhat.com>
---
src/cli/cli-report.c | 9 ++++-----
src/gui-wizard-gtk/wizard.c | 18 ++++++++++--------
src/lib/dump_dir.c | 14 ++++++++++++--
src/lib/report.c | 2 +-
src/plugins/reporter-bugzilla.c | 4 ++--
src/plugins/rhbz.c | 12 ++++++------
src/plugins/rhbz.h | 2 +-
7 files changed, 36 insertions(+), 25 deletions(-)
diff --git a/src/cli/cli-report.c b/src/cli/cli-report.c
index 1e13a38..0e36150 100644
--- a/src/cli/cli-report.c
+++ b/src/cli/cli-report.c
@@ -333,11 +333,10 @@ static int run_report_editor(problem_data_t *problem_data)
return 2;
}
- fseek(fp, 0, SEEK_END);
- unsigned long size = ftell(fp);
- fseek(fp, 0, SEEK_SET);
-
- char *text = (char*)xmalloc(size + 1);
+ off_t size = fstat_st_size_or_die(fileno(fp));
+ if (size > INT_MAX/4)
+ size = INT_MAX/4; /* paranoia */
+ char *text = xmalloc(size + 1);
if (fread(text, 1, size, fp) != size)
{
error_msg("Can't read '%s'", filename);
diff --git a/src/gui-wizard-gtk/wizard.c b/src/gui-wizard-gtk/wizard.c
index 57365bd..4820546 100644
--- a/src/gui-wizard-gtk/wizard.c
+++ b/src/gui-wizard-gtk/wizard.c
@@ -1092,14 +1092,16 @@ static void append_item_to_ls_details(gpointer name, gpointer value, gpointer da
{
struct stat statbuf;
statbuf.st_size = 0;
- stat(item->content, &statbuf);
- stats->filesize += statbuf.st_size;
- char *msg = xasprintf(_("(binary file, %llu bytes)"), (long long)statbuf.st_size);
- gtk_list_store_set(g_ls_details, &iter,
- DETAIL_COLUMN_NAME, (char *)name,
- DETAIL_COLUMN_VALUE, msg,
- -1);
- free(msg);
+ if (stat(item->content, &statbuf) == 0)
+ {
+ stats->filesize += statbuf.st_size;
+ char *msg = xasprintf(_("(binary file, %llu bytes)"), (long long)statbuf.st_size);
+ gtk_list_store_set(g_ls_details, &iter,
+ DETAIL_COLUMN_NAME, (char *)name,
+ DETAIL_COLUMN_VALUE, msg,
+ -1);
+ free(msg);
+ }
}
int cur_value;
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index f2e7376..e1af555 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -287,7 +287,8 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
dir = dd->dd_dirname = rm_trailing_slashes(dir);
struct stat stat_buf;
- stat(dir, &stat_buf);
+ if (stat(dir, &stat_buf) != 0)
+ goto cant_access;
/* & 0666 should remove the executable bit */
dd->mode = (stat_buf.st_mode & 0666);
@@ -331,7 +332,10 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
else
{
if (!(flags & DD_FAIL_QUIETLY_EACCES))
+ {
+ cant_access:
perror_msg("Can't access '%s'", dir);
+ }
}
dd_close(dd);
return NULL;
@@ -549,7 +553,13 @@ void dd_sanitize_mode_and_owner(struct dump_dir *dd)
if (lstat(full_path, &statbuf) == 0 && S_ISREG(statbuf.st_mode))
{
if ((statbuf.st_mode & 0777) != dd->mode)
- chmod(full_path, dd->mode);
+ {
+ if (chmod(full_path, dd->mode) != 0)
+ {
+ perror_msg("Can't change '%s' mode to 0%o", full_path,
+ (unsigned)dd->mode);
+ }
+ }
if (statbuf.st_uid != dd->dd_uid || statbuf.st_gid != dd->dd_gid)
{
if (lchown(full_path, dd->dd_uid, dd->dd_gid) != 0)
diff --git a/src/lib/report.c b/src/lib/report.c
index 9e1d601..7b7deab 100644
--- a/src/lib/report.c
+++ b/src/lib/report.c
@@ -175,7 +175,7 @@ int report_problem_in_memory(problem_data_t *pd, int flags)
dd_close(dd);
VERB2 log("Temp problem dir: '%s'", dir_name);
- if (flags & LIBREPORT_NOWAIT)
+ if (!(flags & LIBREPORT_WAIT))
flags |= LIBREPORT_DEL_DIR;
result = report_problem_in_dir(dir_name, flags);
diff --git a/src/plugins/reporter-bugzilla.c b/src/plugins/reporter-bugzilla.c
index 7bde348..9374505 100644
--- a/src/plugins/reporter-bugzilla.c
+++ b/src/plugins/reporter-bugzilla.c
@@ -165,7 +165,7 @@ int main(int argc, char **argv)
xmlrpc_value *all_bugs = rhbz_get_member("bugs", result);
xmlrpc_DECREF(result);
- int all_bugs_size = rhbz_array_size(all_bugs);
+ unsigned all_bugs_size = rhbz_array_size(all_bugs);
if (all_bugs_size > 0)
{
int bug_id = rhbz_bug_id(all_bugs);
@@ -302,7 +302,7 @@ int main(int argc, char **argv)
if (!all_bugs)
error_msg_and_die(_("Missing mandatory member 'bugs'"));
- int all_bugs_size = rhbz_array_size(all_bugs);
+ unsigned all_bugs_size = rhbz_array_size(all_bugs);
// When someone clones bug it has same duphash, so we can find more than 1.
// Need to be checked if component is same.
VERB3 log("Bugzilla has %i reports with same duphash '%s'",
diff --git a/src/plugins/rhbz.c b/src/plugins/rhbz.c
index 359c15a..300c5c8 100644
--- a/src/plugins/rhbz.c
+++ b/src/plugins/rhbz.c
@@ -154,11 +154,11 @@ static GList *parse_comments(xmlrpc_value *result_xml)
if (!comments_memb)
return NULL;
- int comments_memb_size = rhbz_array_size(comments_memb);
+ unsigned comments_memb_size = rhbz_array_size(comments_memb);
xmlrpc_env env;
xmlrpc_env_init(&env);
- for (int i = 0; i < comments_memb_size; ++i)
+ for (unsigned i = 0; i < comments_memb_size; ++i)
{
xmlrpc_value* item = NULL;
xmlrpc_array_read_item(&env, comments_memb, i, &item);
@@ -312,14 +312,14 @@ xmlrpc_value *rhbz_get_member(const char *member, xmlrpc_value *xml)
* value. So it is usually not worth checking *envP.
* die or return size of array
*/
-int rhbz_array_size(xmlrpc_value *xml)
+unsigned rhbz_array_size(xmlrpc_value *xml)
{
func_entry();
xmlrpc_env env;
xmlrpc_env_init(&env);
- int size = xmlrpc_array_size(&env, xml);
+ unsigned size = xmlrpc_array_size(&env, xml);
if (env.fault_occurred)
abrt_xmlrpc_die(&env);
@@ -419,12 +419,12 @@ GList *rhbz_bug_cc(xmlrpc_value* result_xml)
if (!cc_member)
return NULL;
- int array_size = rhbz_array_size(cc_member);
+ unsigned array_size = rhbz_array_size(cc_member);
VERB3 log("count members on cc %i", array_size);
GList *cc_list = NULL;
- for (int i = 0; i < array_size; ++i)
+ for (unsigned i = 0; i < array_size; ++i)
{
xmlrpc_value* item = NULL;
xmlrpc_array_read_item(&env, cc_member, i, &item);
diff --git a/src/plugins/rhbz.h b/src/plugins/rhbz.h
index 10c37bc..585b23e 100644
--- a/src/plugins/rhbz.h
+++ b/src/plugins/rhbz.h
@@ -93,7 +93,7 @@ xmlrpc_value *rhbz_search_duphash(struct abrt_xmlrpc *ax, const char *component,
xmlrpc_value *rhbz_get_member(const char *member, xmlrpc_value *xml);
-int rhbz_array_size(xmlrpc_value *xml);
+unsigned rhbz_array_size(xmlrpc_value *xml);
int rhbz_bug_id(xmlrpc_value *xml);
--
1.8.1.4
More information about the Crash-catcher
mailing list