Re: [PATCH 3/4] mdadm:triggers core dump when stat2devnm return NULL

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

On Wed, Mar 08 2017, Zhilong Liu wrote:

> ensure that the device should be a block device when uses
> --wait parameter, such as the 'f' and 'd' type file would
> be triggered core dumped.
> ./mdadm --wait /dev/md/, happened core dump.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>
> diff --git a/Monitor.c b/Monitor.c
> index 802a9d9..1900db3 100644
> --- a/Monitor.c
> +++ b/Monitor.c
> @@ -1002,6 +1002,10 @@ int Wait(char *dev)
>  			strerror(errno));
>  		return 2;
>  	}
> +	if ((S_IFMT & stb.st_mode) != S_IFBLK) {
> +		pr_err("%s is not a block device.\n", dev);
> +		return 2;
> +	}
>  	strcpy(devnm, stat2devnm(&stb));

Surely it would be cleaner to do something like:

  tmp = stat2devnm(&stb);
  if (!tmp) {
      pr_err("%s is not a block device.\n", dev);
      return 2;
  }
  strcpy(devnm, tmp);

This makes it more obvious how you have fixed the crash.

>  
>  	while(1) {
> diff --git a/lib.c b/lib.c
> index b640634..7116298 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -89,9 +89,6 @@ char *devid2kname(int devid)
>  
>  char *stat2kname(struct stat *st)
>  {
> -	if ((S_IFMT & st->st_mode) != S_IFBLK)
> -		return NULL;
> -
>  	return devid2kname(st->st_rdev);
>  }

Why are you removing this test?  It has nothing to do with the other
part of the patch.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]