On Wed, Sep 02, 2020 at 03:21:15PM +0800, lixiaokeng wrote:
> In assemble_map func, if add_feature fails and mp->features is
> default value (NULL), STRDUP(mp->features) will cause a seg-fault.
> In addition, f = STRDUP(mp->features) is just used for APPEND().
> We can directly pass mp->features to APPEND().
>
> Here, we firstly check whether mp->features is null.
>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@xxxxxxxxxx>
> Signed-off-by: lixiaokeng <lixiaokeng@xxxxxxxxxx>
> ---
> libmultipath/dmparser.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
> index 482e9d0e..12182bf5 100644
> --- a/libmultipath/dmparser.c
> +++ b/libmultipath/dmparser.c
> @@ -65,7 +65,7 @@ assemble_map (struct multipath * mp, char * params, int len)
> int i, j;
> int minio;
> int nr_priority_groups, initial_pg_nr;
> - char * p, * f;
> + char * p;
> const char *const end = params + len;
> char no_path_retry[] = "queue_if_no_path";
> char retain_hwhandler[] = "retain_attached_hw_handler";
> @@ -86,9 +86,12 @@ assemble_map (struct multipath * mp, char * params, int len)
> get_linux_version_code() < KERNEL_VERSION(4, 3, 0))
> add_feature(&mp->features, retain_hwhandler);
>
> - f = STRDUP(mp->features);
clearly strdup()ing without checking if mp->features NULL is incorrect.
However, I'm not sure that we need to fail if mp->features is NULL.
instead, int the APPEND call, we could use the gcc ternary operator
extension
(mp->features)?: "0"
to use "0" if mp->features is NULL.
Also, have you seen this actually occur? Or is this just a theoretical
issue that you've found from reading the code. It seems like
setup_map() will always call select_features() before calling
assemble_map(), which should mean that mp->features will always be set
in this case. Perhaps I'm missing something here.
-Ben
> + if (!mp->features) {
> + condlog(0, "mp->features is still NULL.");
> + goto err;
> + }
>
> - APPEND(p, end, "%s %s %i %i", f, mp->hwhandler, nr_priority_groups,
> + APPEND(p, end, "%s %s %i %i", mp->features, mp->hwhandler, nr_priority_groups,
> initial_pg_nr);
>
> vector_foreach_slot (mp->pg, pgp, i) {
> @@ -110,12 +113,10 @@ assemble_map (struct multipath * mp, char * params, int len)
> }
> }
>
> - FREE(f);
> condlog(4, "%s: assembled map [%s]", mp->alias, params);
> return 0;
>
> err:
> - FREE(f);
> return 1;
> }
>
> --
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
