* [PATCH V6 00/14] multipath-tools series: some cleanups and fixes checked by codedex tool
@ 2020-09-19 10:27 lixiaokeng
2020-09-19 10:28 ` [PATCH V6 11/14] mpathpersist: check whether malloc paramp->trnptid_list fails in handle_args func lixiaokeng
0 siblings, 1 reply; 3+ messages in thread
From: lixiaokeng @ 2020-09-19 10:27 UTC (permalink / raw)
To: Martin Wilck, Benjamin Marzinski, Christophe Varoqui,
dm-devel mailing list
Cc: linfeilong, liuzhiqiang (I)
Patch 11 is changed.
V5->V6 "paramp->num_transportid = num_transportid" is deleted
in alloc_prout_param_descriptor because paramp->num_transportid
will be set after if (num_transportids) in handle_args.
Zhiqiang Liu (7):
multipathd: check return value of malloc in cli_getprkey func
kpartx: use xmalloc to instead of malloc in main func
libmultipath: check return value of dm_mapname in sysfs_check_holders
libmultipath: donot free *dst if REALLOC fails in merge_words
libmultipath: check whether mpp->features is NUll in setup_map
util/tests: use assert_non_null to ensure malloc returns non-null
pointer
mpathpersist: check whether malloc paramp->trnptid_list fails in
handle_args func
lixiaokeng (7):
multipathd: initialize major and minor in cli_add_map
libmultipath: change malloc to calloc in print_foreign_topology
libmultipath: use map instead of dm_task_get_name
multipathd: check MALLOC return value in mpath_pr_event_handler_fn
libmultipathpersist: use update_multipath_table/status in get_mpvec
multipath: use update_multipath_table/status in check_useable_paths
multipathpersist: delete unused variable in handle_args
kpartx/kpartx.c | 36 +++++++++---------
libmpathpersist/mpath_persist.c | 15 +++-----
libmultipath/configure.c | 5 +++
libmultipath/devmapper.c | 2 +-
libmultipath/dmparser.c | 18 ++++-----
libmultipath/foreign.c | 4 +-
libmultipath/sysfs.c | 6 ++-
mpathpersist/main.c | 66 +++++++++++++++++++++++++--------
multipath/main.c | 9 ++---
multipathd/cli_handlers.c | 4 +-
multipathd/main.c | 8 ++--
tests/util.c | 2 +
12 files changed, 106 insertions(+), 69 deletions(-)
--
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH V6 11/14] mpathpersist: check whether malloc paramp->trnptid_list fails in handle_args func 2020-09-19 10:27 [PATCH V6 00/14] multipath-tools series: some cleanups and fixes checked by codedex tool lixiaokeng @ 2020-09-19 10:28 ` lixiaokeng 2020-09-21 8:56 ` Martin Wilck 0 siblings, 1 reply; 3+ messages in thread From: lixiaokeng @ 2020-09-19 10:28 UTC (permalink / raw) To: Martin Wilck, Benjamin Marzinski, Christophe Varoqui, dm-devel mailing list Cc: linfeilong, liuzhiqiang (I) In handle_args func, we donot check whether malloc paramp and each paramp->trnptid_list[j] fails before using them, it may cause access NULL pointer. Here, we add alloc_prout_param_descriptor to allocate and init paramp, and we add free_prout_param_descriptor to free paramp and each paramp->trnptid_list[j]. We change num_transport to num_transportids to combine them. Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com> Signed-off-by: lixiaokeng <lixiaokeng@huawei.com> --- mpathpersist/main.c | 64 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/mpathpersist/main.c b/mpathpersist/main.c index 28bfe410..f6a8b38c 100644 --- a/mpathpersist/main.c +++ b/mpathpersist/main.c @@ -153,6 +153,37 @@ static int do_batch_file(const char *batch_fn) return ret; } +static struct prout_param_descriptor * +alloc_prout_param_descriptor(int num_transportid) +{ + struct prout_param_descriptor *paramp; + + if (num_transportid < 0 || num_transportid > MPATH_MX_TIDS) + return NULL; + + paramp= malloc(sizeof(struct prout_param_descriptor) + + (sizeof(struct transportid *) * num_transportid)); + + if (!paramp) + return NULL; + + memset(paramp, 0, sizeof(struct prout_param_descriptor) + + (sizeof(struct transportid *) * num_transportid)); + return paramp; +} + +static void free_prout_param_descriptor(struct prout_param_descriptor *paramp) +{ + unsigned int i; + if (!paramp) + return; + + for (i = 0; i < paramp->num_transportid; i++) + free(paramp->trnptid_list[i]); + + free(paramp); +} + static int handle_args(int argc, char * argv[], int nline) { int c; @@ -177,7 +208,6 @@ static int handle_args(int argc, char * argv[], int nline) int prin = 1; int prin_sa = -1; int prout_sa = -1; - int num_transport =0; char *batch_fn = NULL; void *resp = NULL; struct transportid * tmp; @@ -334,13 +364,13 @@ static int handle_args(int argc, char * argv[], int nline) break; case 'X': - if (0 != construct_transportid(optarg, transportids, num_transport)) { + if (0 != construct_transportid(optarg, transportids, num_transportids)) { fprintf(stderr, "bad argument to '--transport-id'\n"); ret = MPATH_PR_SYNTAX_ERROR; goto out; } - ++num_transport; + ++num_transportids; break; case 'l': @@ -525,9 +555,12 @@ static int handle_args(int argc, char * argv[], int nline) int j; struct prout_param_descriptor *paramp; - paramp= malloc(sizeof(struct prout_param_descriptor) + (sizeof(struct transportid *)*(MPATH_MX_TIDS ))); - - memset(paramp, 0, sizeof(struct prout_param_descriptor) + (sizeof(struct transportid *)*(MPATH_MX_TIDS))); + paramp = alloc_prout_param_descriptor(num_transportids); + if (!paramp) { + fprintf(stderr, "malloc paramp failed\n"); + ret = MPATH_PR_OTHER; + goto out_fd; + } for (j = 7; j >= 0; --j) { paramp->key[j] = (param_rk & 0xff); @@ -544,13 +577,19 @@ static int handle_args(int argc, char * argv[], int nline) if (param_aptpl) paramp->sa_flags |= MPATH_F_APTPL_MASK; - if (num_transport) + if (num_transportids) { paramp->sa_flags |= MPATH_F_SPEC_I_PT_MASK; - paramp->num_transportid = num_transport; - for (j = 0 ; j < num_transport; j++) + paramp->num_transportid = num_transportids; + for (j = 0 ; j < num_transportids; j++) { paramp->trnptid_list[j] = (struct transportid *)malloc(sizeof(struct transportid)); + if (!paramp->trnptid_list[j]) { + fprintf(stderr, "malloc paramp->trnptid_list[%d] failed.\n", j); + ret = MPATH_PR_OTHER; + free_prout_param_descriptor(paramp); + goto out_fd; + } memcpy(paramp->trnptid_list[j], &transportids[j],sizeof(struct transportid)); } } @@ -558,12 +597,7 @@ static int handle_args(int argc, char * argv[], int nline) /* PROUT commands other than 'register and move' */ ret = __mpath_persistent_reserve_out (fd, prout_sa, 0, prout_type, paramp, noisy); - for (j = 0 ; j < num_transport; j++) - { - tmp = paramp->trnptid_list[j]; - free(tmp); - } - free(paramp); + free_prout_param_descriptor(paramp); } if (ret != MPATH_PR_SUCCESS) -- ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V6 11/14] mpathpersist: check whether malloc paramp->trnptid_list fails in handle_args func 2020-09-19 10:28 ` [PATCH V6 11/14] mpathpersist: check whether malloc paramp->trnptid_list fails in handle_args func lixiaokeng @ 2020-09-21 8:56 ` Martin Wilck 0 siblings, 0 replies; 3+ messages in thread From: Martin Wilck @ 2020-09-21 8:56 UTC (permalink / raw) To: lixiaokeng, Benjamin Marzinski, Christophe Varoqui, dm-devel mailing list Cc: linfeilong, liuzhiqiang (I) On Sat, 2020-09-19 at 18:28 +0800, lixiaokeng wrote: > In handle_args func, we donot check whether malloc paramp and > each paramp->trnptid_list[j] fails before using them, it may > cause access NULL pointer. > > Here, we add alloc_prout_param_descriptor to allocate and init > paramp, and we add free_prout_param_descriptor to free paramp > and each paramp->trnptid_list[j]. > > We change num_transport to num_transportids to combine them. > > Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com> > Signed-off-by: lixiaokeng <lixiaokeng@huawei.com> > --- > mpathpersist/main.c | 64 ++++++++++++++++++++++++++++++++++--------- > -- > 1 file changed, 49 insertions(+), 15 deletions(-) > OK. So your v5 patch wasn't wrong either, as you did set num_transportids twice. I admit I missed that. But it's better now anyway. Actually, I would have preferred if you solved the issue the other way around, by moving the assignment to num_transportids to after the memset() in alloc_prout_param_descriptor(). But it's not worth forcing you into yet another iteration, so: Reviewed-by: Martin Wilck <mwilck@suse.com> This means the whole series (except 8/14 which had been taken back) is acked now, and I'll push it to the upstream-queue branch. Martin ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-21 8:56 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-19 10:27 [PATCH V6 00/14] multipath-tools series: some cleanups and fixes checked by codedex tool lixiaokeng 2020-09-19 10:28 ` [PATCH V6 11/14] mpathpersist: check whether malloc paramp->trnptid_list fails in handle_args func lixiaokeng 2020-09-21 8:56 ` Martin Wilck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).