From cbce58135341d470c3a57e343bebe253384e1198 Mon Sep 17 00:00:00 2001 From: Don Brady Date: Tue, 20 Feb 2018 12:19:42 -0700 Subject: [PATCH] Fix coverity defects: zfs channel programs CID 173243, 173245: Memory - corruptions (OVERRUN) Added size argument to lcompat_sprintf() to avoid use of INT_MAX CID 173244: Integer handling issues (OVERFLOW_BEFORE_WIDEN) Added cast to uint64_t to avoid a 32 bit overflow warning CID 173242: Integer handling issues (CONSTANT_EXPRESSION_RESULT) Conditionally removed unused luai_numisnan() floating point check CID 173241: Resource leaks (RESOURCE_LEAK) Added missing close(fd) on error path CID 173240: (UNINIT) Fixed uninitialized variable in get_special_prop() CID 147560: Null pointer dereferences (NULL_RETURNS) Cleaned up bad code merge in dsl_dataset_promote_check() CID 28475: Memory - illegal accesses (OVERRUN) Fixed lcompat_sprintf() to use a size paramater CID 28418, 28422: Error handling issues (CHECKED_RETURN) Added function result cast to (void) to avoid warning CID 23935, 28411, 28412: Memory - corruptions (ARRAY_VS_SINGLETON) Added casts to avoid exposing result as an array Reviewed-by: Brian Behlendorf Signed-off-by: Don Brady Closes #7181 --- cmd/zfs/zfs_main.c | 2 ++ include/sys/lua/luaconf.h | 7 ++++--- module/lua/lapi.c | 2 +- module/lua/lcompat.c | 4 ++-- module/lua/lctype.h | 2 +- module/lua/llex.c | 2 +- module/lua/lobject.c | 2 +- module/lua/lparser.c | 2 +- module/lua/lstring.c | 6 ++++-- module/lua/ltable.c | 2 ++ module/lua/ltable.h | 2 +- module/zfs/dsl_dataset.c | 5 +---- module/zfs/zcp_get.c | 2 +- module/zfs/zcp_synctask.c | 3 ++- 14 files changed, 24 insertions(+), 19 deletions(-) diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index 16410d2f23..b9c3a5cf3f 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -7275,6 +7275,8 @@ zfs_do_channel_program(int argc, char **argv) if ((zhp = zpool_open(g_zfs, poolname)) == NULL) { (void) fprintf(stderr, gettext("cannot open pool '%s'"), poolname); + if (fd != 0) + (void) close(fd); return (1); } zpool_close(zhp); diff --git a/include/sys/lua/luaconf.h b/include/sys/lua/luaconf.h index 835d9ccfe1..302c57a8c4 100644 --- a/include/sys/lua/luaconf.h +++ b/include/sys/lua/luaconf.h @@ -12,7 +12,7 @@ #include -extern ssize_t lcompat_sprintf(char *, const char *, ...); +extern ssize_t lcompat_sprintf(char *, size_t size, const char *, ...); extern int64_t lcompat_strtoll(const char *, char **); extern int64_t lcompat_pow(int64_t, int64_t); @@ -402,9 +402,10 @@ extern int64_t lcompat_pow(int64_t, int64_t); #define PRId64 "lld" #endif -#define LUA_NUMBER_FMT "%" PRId64 -#define lua_number2str(s,n) lcompat_sprintf((s), LUA_NUMBER_FMT, (n)) #define LUAI_MAXNUMBER2STR 32 /* 16 digits, sign, point, and \0 */ +#define LUA_NUMBER_FMT "%" PRId64 +#define lua_number2str(s,n) \ + lcompat_sprintf((s), LUAI_MAXNUMBER2STR, LUA_NUMBER_FMT, (n)) /* diff --git a/module/lua/lapi.c b/module/lua/lapi.c index c8b49dc632..54c74d38e9 100644 --- a/module/lua/lapi.c +++ b/module/lua/lapi.c @@ -424,7 +424,7 @@ LUA_API lua_CFunction lua_tocfunction (lua_State *L, int idx) { LUA_API void *lua_touserdata (lua_State *L, int idx) { StkId o = index2addr(L, idx); switch (ttypenv(o)) { - case LUA_TUSERDATA: return (rawuvalue(o) + 1); + case LUA_TUSERDATA: return ((void *)(rawuvalue(o) + 1)); case LUA_TLIGHTUSERDATA: return pvalue(o); default: return NULL; } diff --git a/module/lua/lcompat.c b/module/lua/lcompat.c index 3fa2e664ca..c0a27182c7 100644 --- a/module/lua/lcompat.c +++ b/module/lua/lcompat.c @@ -6,13 +6,13 @@ ssize_t -lcompat_sprintf(char *buf, const char *fmt, ...) +lcompat_sprintf(char *buf, size_t size, const char *fmt, ...) { ssize_t res; va_list args; va_start(args, fmt); - res = vsnprintf(buf, INT_MAX, fmt, args); + res = vsnprintf(buf, size, fmt, args); va_end(args); return (res); diff --git a/module/lua/lctype.h b/module/lua/lctype.h index b53d883704..b16b6bc7da 100644 --- a/module/lua/lctype.h +++ b/module/lua/lctype.h @@ -48,7 +48,7 @@ /* ** add 1 to char to allow index -1 (EOZ) */ -#define testprop(c,p) (luai_ctype_[(c)+1] & (p)) +#define testprop(c,p) (luai_ctype_[(lu_byte)(c)+1] & (p)) /* ** 'lalpha' (Lua alphabetic) and 'lalnum' (Lua alphanumeric) both include '_' diff --git a/module/lua/llex.c b/module/lua/llex.c index a7144681c6..e98d74320d 100644 --- a/module/lua/llex.c +++ b/module/lua/llex.c @@ -235,7 +235,7 @@ static void read_numeral (LexState *ls, SemInfo *seminfo) { expo = "Pp"; for (;;) { if (check_next(ls, expo)) /* exponent part? */ - check_next(ls, "+-"); /* optional exponent sign */ + (void) check_next(ls, "+-"); /* optional exponent sign */ if (lisxdigit(ls->current) || ls->current == '.') save_and_next(ls); else break; diff --git a/module/lua/lobject.c b/module/lua/lobject.c index 7c0b3133a4..024d3199fe 100644 --- a/module/lua/lobject.c +++ b/module/lua/lobject.c @@ -201,7 +201,7 @@ const char *luaO_pushvfstring (lua_State *L, const char *fmt, va_list argp) { } case 'p': { char buff[4*sizeof(void *) + 8]; /* should be enough space for a `%p' */ - int l = lcompat_sprintf(buff, "%p", va_arg(argp, void *)); + int l = lcompat_sprintf(buff, sizeof(buff), "%p", va_arg(argp, void *)); pushstr(L, buff, l); break; } diff --git a/module/lua/lparser.c b/module/lua/lparser.c index 775e6ed25c..e1dd88f2f6 100644 --- a/module/lua/lparser.c +++ b/module/lua/lparser.c @@ -1527,7 +1527,7 @@ static void retstat (LexState *ls) { } } luaK_ret(fs, first, nret); - testnext(ls, ';'); /* skip optional semicolon */ + (void) testnext(ls, ';'); /* skip optional semicolon */ } diff --git a/module/lua/lstring.c b/module/lua/lstring.c index 2903d2b754..7fcef3d88a 100644 --- a/module/lua/lstring.c +++ b/module/lua/lstring.c @@ -97,14 +97,16 @@ void luaS_resize (lua_State *L, int newsize) { static TString *createstrobj (lua_State *L, const char *str, size_t l, int tag, unsigned int h, GCObject **list) { TString *ts; + char *sbuf; size_t totalsize; /* total size of TString object */ totalsize = sizeof(TString) + ((l + 1) * sizeof(char)); ts = &luaC_newobj(L, tag, totalsize, list, 0)->ts; ts->tsv.len = l; ts->tsv.hash = h; ts->tsv.extra = 0; - memcpy(ts+1, str, l*sizeof(char)); - ((char *)(ts+1))[l] = '\0'; /* ending 0 */ + sbuf = (char *)(TString *)(ts + 1); + memcpy(sbuf, str, l*sizeof(char)); + sbuf[l] = '\0'; /* ending 0 */ return ts; } diff --git a/module/lua/ltable.c b/module/lua/ltable.c index 73e1c25415..ea5a43c787 100644 --- a/module/lua/ltable.c +++ b/module/lua/ltable.c @@ -405,8 +405,10 @@ static Node *getfreepos (Table *t) { TValue *luaH_newkey (lua_State *L, Table *t, const TValue *key) { Node *mp; if (ttisnil(key)) luaG_runerror(L, "table index is nil"); +#if defined LUA_HAS_FLOAT_NUMBERS else if (ttisnumber(key) && luai_numisnan(L, nvalue(key))) luaG_runerror(L, "table index is NaN"); +#endif mp = mainposition(t, key); if (!ttisnil(gval(mp)) || isdummy(mp)) { /* main position is taken? */ Node *othern; diff --git a/module/lua/ltable.h b/module/lua/ltable.h index 212500011f..ea877ebf4e 100644 --- a/module/lua/ltable.h +++ b/module/lua/ltable.h @@ -11,7 +11,7 @@ #include "lobject.h" -#define gnode(t,i) (&(t)->node[i]) +#define gnode(t,i) ((Node *)&(t)->node[i]) #define gkey(n) (&(n)->i_key.tvk) #define gval(n) (&(n)->i_val) #define gnext(n) ((n)->i_key.nk.next) diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index 86f0888890..40c2df1e7e 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -2750,12 +2750,8 @@ dsl_dataset_promote_check(void *arg, dmu_tx_t *tx) return (err); hds = ddpa->ddpa_clone; - snap = list_head(&ddpa->shared_snaps); - origin_ds = snap->ds; max_snap_len = MAXNAMELEN - strlen(ddpa->ddpa_clonename) - 1; - snap = list_head(&ddpa->origin_snaps); - if (dsl_dataset_phys(hds)->ds_flags & DS_FLAG_NOPROMOTE) { promote_rele(ddpa, FTAG); return (SET_ERROR(EXDEV)); @@ -2789,6 +2785,7 @@ dsl_dataset_promote_check(void *arg, dmu_tx_t *tx) /* compute origin's new unique space */ snap = list_tail(&ddpa->clone_snaps); + ASSERT(snap != NULL); ASSERT3U(dsl_dataset_phys(snap->ds)->ds_prev_snap_obj, ==, origin_ds->ds_object); dsl_deadlist_space_range(&snap->ds->ds_deadlist, diff --git a/module/zfs/zcp_get.c b/module/zfs/zcp_get.c index 7645bc1585..2481bb1fe2 100644 --- a/module/zfs/zcp_get.c +++ b/module/zfs/zcp_get.c @@ -303,7 +303,7 @@ get_special_prop(lua_State *state, dsl_dataset_t *ds, const char *dsname, { int error = 0; objset_t *os; - uint64_t numval; + uint64_t numval = 0; char *strval = kmem_alloc(ZAP_MAXVALUELEN, KM_SLEEP); char setpoint[ZFS_MAX_DATASET_NAME_LEN] = "Internal error - setpoint not determined"; diff --git a/module/zfs/zcp_synctask.c b/module/zfs/zcp_synctask.c index 5a70664073..196a3d4b75 100644 --- a/module/zfs/zcp_synctask.c +++ b/module/zfs/zcp_synctask.c @@ -297,7 +297,8 @@ zcp_synctask_wrapper(lua_State *state) dsl_pool_t *dp = ri->zri_pool; /* MOS space is triple-dittoed, so we multiply by 3. */ - uint64_t funcspace = (info->blocks_modified << DST_AVG_BLKSHIFT) * 3; + uint64_t funcspace = + ((uint64_t)info->blocks_modified << DST_AVG_BLKSHIFT) * 3; zcp_parse_args(state, info->name, info->pargs, info->kwargs);