[Skiboot] [PATCH] vpd: Sanitize VPD data

[ppaidipe]

On 2018-06-24 16:28, Vasant Hegde wrote:
> On OpenPower system, VPD keyword size tells us the maximum size of the 
> data.
> But they fill trailing end with space (0x20) instead of NULL. Also spec
> doesn't stop user to have space (0x20) within actual data.
> 
> This patch discards trailing spaces before populating device tree.
> 
> Reported-by: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
> ---
>  hdata/vpd.c | 45 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
> 


Tested-by: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>


> diff --git a/hdata/vpd.c b/hdata/vpd.c
> index 038569af0..5379a67b4 100644
> --- a/hdata/vpd.c
> +++ b/hdata/vpd.c
> @@ -22,6 +22,7 @@
>  #include <device.h>
>  #include "hdata.h"
>  #include <inttypes.h>
> +#include <mem_region-malloc.h>
> 
>  struct card_info {
>  	const char *ccin; 	/* Customer card identification number */
> @@ -221,6 +222,32 @@ static const struct card_info 
> *card_info_lookup(char *ccin)
>  	return NULL;
>  }
> 
> +/* Discard trailing spaces and populate device tree */
> +static struct dt_property *dt_add_prop_sanitize_val(struct dt_node 
> *node,
> +			     const char *name, const char *val, int vlen)
> +{
> +	char *prop = zalloc(vlen + 1);
> +	int i;
> +	struct dt_property *p = NULL;
> +
> +	if (!prop)
> +		return p;
> +
> +	memcpy(prop, val, vlen);
> +	for (i = vlen - 1; i >= 0; i--) {
> +		if (prop[i] != 0x20) {
> +			prop[i + 1] = '\0';
> +			break;
> +		}
> +	}
> +
> +	if (i >= 0)
> +		p = dt_add_property_string(node, name, prop);
> +
> +	free(prop);
> +	return p;
> +}
> +
>  /*
>   * For OpenPOWER, we only decipher OPFR records. While OP HDAT have 
> VINI
>   * records too, populating the fields in there is optional. Also, 
> there
> @@ -235,27 +262,27 @@ static void vpd_opfr_parse(struct dt_node *node,
>  	/* Vendor Name */
>  	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "VN", &sz);
>  	if (kw)
> -		dt_add_property_nstr(node, "vendor", kw, sz);
> +		dt_add_prop_sanitize_val(node, "vendor", kw, sz);
> 
>  	/* FRU Description */
>  	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "DR", &sz);
>  	if (kw)
> -		dt_add_property_nstr(node, "description", kw, sz);
> +		dt_add_prop_sanitize_val(node, "description", kw, sz);
> 
>  	/* Part number */
>  	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "VP", &sz);
>  	if (kw)
> -		dt_add_property_nstr(node, "part-number", kw, sz);
> +		dt_add_prop_sanitize_val(node, "part-number", kw, sz);
> 
>  	/* Serial number */
>  	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "VS", &sz);
>  	if (kw)
> -		dt_add_property_nstr(node, "serial-number", kw, sz);
> +		dt_add_prop_sanitize_val(node, "serial-number", kw, sz);
> 
>  	/* Build date in BCD */
>  	kw = vpd_find(fruvpd, fruvpd_sz, "OPFR", "MB", &sz);
>  	if (kw)
> -		dt_add_property_nstr(node, "build-date", kw, sz);
> +		dt_add_prop_sanitize_val(node, "build-date", kw, sz);
> 
>  	return;
>  }
> @@ -272,12 +299,12 @@ static void vpd_vrml_parse(struct dt_node *node,
>  	/* Part number */
>  	kw = vpd_find(fruvpd, fruvpd_sz, "VRML", "PN", &sz);
>  	if (kw)
> -		dt_add_property_nstr(node, "part-number", kw, sz);
> +		dt_add_prop_sanitize_val(node, "part-number", kw, sz);
> 
>  	/* Serial number */
>  	kw = vpd_find(fruvpd, fruvpd_sz, "VRML", "SN", &sz);
>  	if (kw)
> -		dt_add_property_nstr(node, "serial-number", kw, sz);
> +		dt_add_prop_sanitize_val(node, "serial-number", kw, sz);
> 
>  	return;
>  }
> @@ -548,13 +575,13 @@ static void sysvpd_parse_opp(const void *sysvpd,
> unsigned int sysvpd_sz)
> 
>  	v = vpd_find(sysvpd, sysvpd_sz, "OSYS", "MM", &sz);
>  	if (v)
> -		dt_add_property_nstr(dt_root, "model", v, sz);
> +		dt_add_prop_sanitize_val(dt_root, "model", v, sz);
>  	else
>  		dt_add_property_string(dt_root, "model", "Unknown");
> 
>  	v = vpd_find(sysvpd, sysvpd_sz, "OSYS", "SS", &sz);
>  	if (v)
> -		dt_add_property_nstr(dt_root, "system-id", v, sz);
> +		dt_add_prop_sanitize_val(dt_root, "system-id", v, sz);
>  	else
>  		dt_add_property_string(dt_root, "system-id", "Unknown");
>  }