diff mbox series

[08/21] libmultipath (coverity): fix tainted values in alua_rtpg.c

Message ID 20211118231338.22358-9-mwilck@suse.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show
Series multipath-tools: coverity fixes | expand

Commit Message

Martin Wilck Nov. 18, 2021, 11:13 p.m. UTC 
From: Martin Wilck <mwilck@suse.com>

Coverity needs tainted values limited by constant expressions.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/prioritizers/alua_rtpg.c | 13 ++++++------
 libmultipath/prioritizers/alua_spc3.h | 30 ++++++++++++++++++++++-----
 2 files changed, 31 insertions(+), 12 deletions(-)

Comments

Benjamin Marzinski Nov. 30, 2021, 1:23 a.m. UTC | #1 
On Fri, Nov 19, 2021 at 12:13:25AM +0100, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> Coverity needs tainted values limited by constant expressions.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/prioritizers/alua_rtpg.c | 13 ++++++------
>  libmultipath/prioritizers/alua_spc3.h | 30 ++++++++++++++++++++++-----
>  2 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/libmultipath/prioritizers/alua_rtpg.c b/libmultipath/prioritizers/alua_rtpg.c
> index 420a2e3..3f9c0e7 100644
> --- a/libmultipath/prioritizers/alua_rtpg.c
> +++ b/libmultipath/prioritizers/alua_rtpg.c
> @@ -27,7 +27,6 @@
>  #include "../structs.h"
>  #include "../prio.h"
>  #include "../discovery.h"
> -#include "../unaligned.h"
>  #include "../debug.h"
>  #include "alua_rtpg.h"
>  
> @@ -252,12 +251,12 @@ int
>  get_target_port_group(const struct path * pp, unsigned int timeout)
>  {
>  	unsigned char		*buf;
> -	struct vpd83_data *	vpd83;
> -	struct vpd83_dscr *	dscr;
> +	const struct vpd83_data *	vpd83;
> +	const struct vpd83_dscr *	dscr;
>  	int			rc;
>  	int			buflen, scsi_buflen;
>  
> -	buflen = 4096;
> +	buflen = VPD_BUFLEN;
>  	buf = (unsigned char *)malloc(buflen);
>  	if (!buf) {
>  		PRINT_DEBUG("malloc failed: could not allocate"
> @@ -298,13 +297,13 @@ get_target_port_group(const struct path * pp, unsigned int timeout)
>  	rc = -RTPG_NO_TPG_IDENTIFIER;
>  	FOR_EACH_VPD83_DSCR(vpd83, dscr) {
>  		if (vpd83_dscr_istype(dscr, IDTYPE_TARGET_PORT_GROUP)) {
> -			struct vpd83_tpg_dscr *p;
> +			const struct vpd83_tpg_dscr *p;
>  			if (rc != -RTPG_NO_TPG_IDENTIFIER) {
>  				PRINT_DEBUG("get_target_port_group: more "
>  					    "than one TPG identifier found!");
>  				continue;
>  			}
> -			p  = (struct vpd83_tpg_dscr *)dscr->data;
> +			p  = (const struct vpd83_tpg_dscr *)dscr->data;
>  			rc = get_unaligned_be16(p->tpg);
>  		}
>  	}
> @@ -377,7 +376,7 @@ get_asymmetric_access_state(const struct path *pp, unsigned int tpg,
>  	uint64_t		scsi_buflen;
>  	int fd = pp->fd;
>  
> -	buflen = 4096;
> +	buflen = VPD_BUFLEN;
>  	buf = (unsigned char *)malloc(buflen);
>  	if (!buf) {
>  		PRINT_DEBUG ("malloc failed: could not allocate"
> diff --git a/libmultipath/prioritizers/alua_spc3.h b/libmultipath/prioritizers/alua_spc3.h
> index 7ba2cf4..611d67e 100644
> --- a/libmultipath/prioritizers/alua_spc3.h
> +++ b/libmultipath/prioritizers/alua_spc3.h
> @@ -14,6 +14,7 @@
>   */
>  #ifndef __SPC3_H__
>  #define __SPC3_H__
> +#include "../unaligned.h"
>  
>  /*=============================================================================
>   * Definitions to support the standard inquiry command as defined in SPC-3.
> @@ -177,7 +178,7 @@ struct vpd83_dscr {
>  } __attribute__((packed));
>  
>  static inline int
> -vpd83_dscr_istype(struct vpd83_dscr *d, unsigned char type)
> +vpd83_dscr_istype(const struct vpd83_dscr *d, unsigned char type)
>  {
>  	return ((d->b1 & 7) == type);
>  }
> @@ -190,6 +191,26 @@ struct vpd83_data {
>  	struct vpd83_dscr	data[0];
>  } __attribute__((packed));
>  
> +#define VPD_BUFLEN 4096
> +
> +static inline unsigned int vpd83_length(const struct vpd83_data *p)
> +{
> +	uint16_t len = get_unaligned_be16(p->length);
> +
> +	return len <= VPD_BUFLEN ? len : VPD_BUFLEN;

Isn't p->length the length of the page data after the header?  So
you either need to add 4 to len, or use (VPD_BUFLEN - 4)

> +}
> +
> +static inline const struct vpd83_dscr *
> +vpd83_next_dscr(const struct vpd83_dscr *d, const struct vpd83_data *p)
> +{
> +	uint16_t pg_len = vpd83_length(p);
> +	ptrdiff_t offs = ((const char *)d - (const char *)p) + d->length + 4;
> +
> +	offs = offs < 0 ? 0 : offs > pg_len ? pg_len : offs;
> +	return (const struct vpd83_dscr *)((const char *)p + offs);
> +}
> +
> +
>  /*-----------------------------------------------------------------------------
>   * This macro should be used to walk through all identification descriptors
>   * defined in the code page 0x83.
> @@ -200,10 +221,9 @@ struct vpd83_data {
>  #define FOR_EACH_VPD83_DSCR(p, d) \
>  		for( \
>  			d = p->data; \
> -			(((char *) d) - ((char *) p)) < \
> -			get_unaligned_be16(p->length); \
> -			d = (struct vpd83_dscr *) \
> -				((char *) d + d->length + 4) \
> +			(((const char *) d) - ((const char *) p)) \

Unless you are adding 4 in vpd83_length(), shouldn't this be:

			(((const char *) d) - ((const char *) p->data)) \

> +				< (ptrdiff_t)vpd83_length(p);	  \
> +			d = vpd83_next_dscr(d, p) \
>  		)
>  
>  /*=============================================================================
> -- 
> 2.33.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
diff mbox series

Patch

diff --git a/libmultipath/prioritizers/alua_rtpg.c b/libmultipath/prioritizers/alua_rtpg.c
index 420a2e3..3f9c0e7 100644
--- a/libmultipath/prioritizers/alua_rtpg.c
+++ b/libmultipath/prioritizers/alua_rtpg.c
@@ -27,7 +27,6 @@ 
 #include "../structs.h"
 #include "../prio.h"
 #include "../discovery.h"
-#include "../unaligned.h"
 #include "../debug.h"
 #include "alua_rtpg.h"
 
@@ -252,12 +251,12 @@  int
 get_target_port_group(const struct path * pp, unsigned int timeout)
 {
 	unsigned char		*buf;
-	struct vpd83_data *	vpd83;
-	struct vpd83_dscr *	dscr;
+	const struct vpd83_data *	vpd83;
+	const struct vpd83_dscr *	dscr;
 	int			rc;
 	int			buflen, scsi_buflen;
 
-	buflen = 4096;
+	buflen = VPD_BUFLEN;
 	buf = (unsigned char *)malloc(buflen);
 	if (!buf) {
 		PRINT_DEBUG("malloc failed: could not allocate"
@@ -298,13 +297,13 @@  get_target_port_group(const struct path * pp, unsigned int timeout)
 	rc = -RTPG_NO_TPG_IDENTIFIER;
 	FOR_EACH_VPD83_DSCR(vpd83, dscr) {
 		if (vpd83_dscr_istype(dscr, IDTYPE_TARGET_PORT_GROUP)) {
-			struct vpd83_tpg_dscr *p;
+			const struct vpd83_tpg_dscr *p;
 			if (rc != -RTPG_NO_TPG_IDENTIFIER) {
 				PRINT_DEBUG("get_target_port_group: more "
 					    "than one TPG identifier found!");
 				continue;
 			}
-			p  = (struct vpd83_tpg_dscr *)dscr->data;
+			p  = (const struct vpd83_tpg_dscr *)dscr->data;
 			rc = get_unaligned_be16(p->tpg);
 		}
 	}
@@ -377,7 +376,7 @@  get_asymmetric_access_state(const struct path *pp, unsigned int tpg,
 	uint64_t		scsi_buflen;
 	int fd = pp->fd;
 
-	buflen = 4096;
+	buflen = VPD_BUFLEN;
 	buf = (unsigned char *)malloc(buflen);
 	if (!buf) {
 		PRINT_DEBUG ("malloc failed: could not allocate"
diff --git a/libmultipath/prioritizers/alua_spc3.h b/libmultipath/prioritizers/alua_spc3.h
index 7ba2cf4..611d67e 100644
--- a/libmultipath/prioritizers/alua_spc3.h
+++ b/libmultipath/prioritizers/alua_spc3.h
@@ -14,6 +14,7 @@ 
  */
 #ifndef __SPC3_H__
 #define __SPC3_H__
+#include "../unaligned.h"
 
 /*=============================================================================
  * Definitions to support the standard inquiry command as defined in SPC-3.
@@ -177,7 +178,7 @@  struct vpd83_dscr {
 } __attribute__((packed));
 
 static inline int
-vpd83_dscr_istype(struct vpd83_dscr *d, unsigned char type)
+vpd83_dscr_istype(const struct vpd83_dscr *d, unsigned char type)
 {
 	return ((d->b1 & 7) == type);
 }
@@ -190,6 +191,26 @@  struct vpd83_data {
 	struct vpd83_dscr	data[0];
 } __attribute__((packed));
 
+#define VPD_BUFLEN 4096
+
+static inline unsigned int vpd83_length(const struct vpd83_data *p)
+{
+	uint16_t len = get_unaligned_be16(p->length);
+
+	return len <= VPD_BUFLEN ? len : VPD_BUFLEN;
+}
+
+static inline const struct vpd83_dscr *
+vpd83_next_dscr(const struct vpd83_dscr *d, const struct vpd83_data *p)
+{
+	uint16_t pg_len = vpd83_length(p);
+	ptrdiff_t offs = ((const char *)d - (const char *)p) + d->length + 4;
+
+	offs = offs < 0 ? 0 : offs > pg_len ? pg_len : offs;
+	return (const struct vpd83_dscr *)((const char *)p + offs);
+}
+
+
 /*-----------------------------------------------------------------------------
  * This macro should be used to walk through all identification descriptors
  * defined in the code page 0x83.
@@ -200,10 +221,9 @@  struct vpd83_data {
 #define FOR_EACH_VPD83_DSCR(p, d) \
 		for( \
 			d = p->data; \
-			(((char *) d) - ((char *) p)) < \
-			get_unaligned_be16(p->length); \
-			d = (struct vpd83_dscr *) \
-				((char *) d + d->length + 4) \
+			(((const char *) d) - ((const char *) p)) \
+				< (ptrdiff_t)vpd83_length(p);	  \
+			d = vpd83_next_dscr(d, p) \
 		)
 
 /*=============================================================================