[dm-devel] [PATCH 1/5] Fix potential null pointer dereference

[Benjamin Marzinski]

On Tue, Nov 16, 2021 at 09:59:14PM +0800, lixiaokeng wrote:
> udev_device_* may return NULL, check it.
> 
> Signed-off-by: Lixiaokeng <lixiaokeng at huawei.com>
> ---
>  libmultipath/discovery.c    |  8 +++++---
>  libmultipath/foreign/nvme.c |  4 +++-
>  libmultipath/util.c         | 10 +++++++++-
>  3 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index f25fe9e3..48f3d8b2 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -388,8 +388,10 @@ sysfs_get_tgt_nodename(struct path *pp, char *node)
>  		if (value && !strcmp(value, "usb")) {
>  			pp->sg_id.proto_id = SCSI_PROTOCOL_USB;
>  			tgtname = udev_device_get_sysname(tgtdev);
> -			strlcpy(node, tgtname, NODE_NAME_SIZE);
> -			return 0;
> +			if (!tgtname) {

I assume that you mean "if (tgtname)"

> +				strlcpy(node, tgtname, NODE_NAME_SIZE);
> +				return 0;
> +			}
>  		}
>  		tgtdev = udev_device_get_parent(tgtdev);
>  	}
> @@ -803,7 +805,7 @@ sysfs_set_nexus_loss_tmo(struct multipath *mpp, struct path *pp)
>  	     parent = udev_device_get_parent(parent)) {
>  		const char *ed = udev_device_get_sysname(parent);
> 
> -		if (!strncmp(ed, ed_str, sizeof(ed_str) - 1)) {
> +		if (ed && !strncmp(ed, ed_str, sizeof(ed_str) - 1)) {
>  			end_dev_id = ed;
>  			break;
>  		}
> diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
> index d40c0869..f778410a 100644
> --- a/libmultipath/foreign/nvme.c
> +++ b/libmultipath/foreign/nvme.c
> @@ -184,7 +184,9 @@ static int snprint_nvme_map(const struct gen_multipath *gmp,
>  							      "firmware_rev"));
>  	case 'r':
>  		val = udev_device_get_sysattr_value(nvm->udev, "ro");
> -		if (val[0] == 1)
> +		if (!val)
> +			return -1;

sprint_ro() returns "undef" for a similar case. Perhaps we should do
that here as well.

-Ben

> +		else if (val[0] == 1)
>  			return append_strbuf_str(buff, "ro");
>  		else
>  			return append_strbuf_str(buff, "rw");
> diff --git a/libmultipath/util.c b/libmultipath/util.c
> index ea858409..3d036e19 100644
> --- a/libmultipath/util.c
> +++ b/libmultipath/util.c
> @@ -168,6 +168,7 @@ size_t strlcat(char * restrict dst, const char * restrict src, size_t size)
>  int devt2devname(char *devname, int devname_len, const char *devt)
>  {
>  	struct udev_device *u_dev;
> +	const char * dev_name;
>  	int r;
> 
>  	if (!devname || !devname_len || !devt)
> @@ -178,7 +179,14 @@ int devt2devname(char *devname, int devname_len, const char *devt)
>  		condlog(0, "\"%s\": invalid major/minor numbers, not found in sysfs", devt);
>  		return 1;
>  	}
> -	r = strlcpy(devname, udev_device_get_sysname(u_dev), devname_len);
> +
> +	dev_name = udev_device_get_sysname(u_dev);
> +	if (!dev_name) {
> +		condlog(2, "\"%s\": fail to get sysname\n", devt);
> +		udev_device_unref(u_dev);
> +		return 1;
> +	}
> +	r = strlcpy(devname, dev_name, devname_len);
>  	udev_device_unref(u_dev);
> 
>  	return !(r < devname_len);
> --