[igt-dev] [PATCH i-g-t] lib/igt_device_scan: Assert if there's empty PCI_SLOT_NAME

[Kamil Konieczny]

On 2022-07-28 at 09:00:24 +0200, Zbigniew Kempczyński wrote:
> On one of our testing machines executed i915_pm_rpm at module-reload subtest
> catches SIGSEGV with following callstack:
> 
> Received signal SIGSEGV.
> Stack trace:

You could put here a few relevant lines, but it's not blocker.

Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

> 
> There's single suspicious code line in devtype_intel() which might trigger
> this behavior - it is strncmp(). But that means pci_slot variable would be
> null or point to some garbage. This variable is initialized before from
> uevent property so lets verify and assert if there's no PCI_SLOT_NAME
> passed from the kernel for pci device. Additionally dump all properties
> and attributes to get bigger picture what comes from the kernel to device
> scanning library.
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Petri Latvala <petri.latvala at intel.com>
> ---
>  lib/igt_device_scan.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index aad7966764..d6fae0650c 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -558,6 +558,16 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
>  #define is_drm_subsystem(dev)  (strequal(get_prop_subsystem(dev), "drm"))
>  #define is_pci_subsystem(dev)  (strequal(get_prop_subsystem(dev), "pci"))
>  
> +static void print_ht(GHashTable *ht);
> +static void dump_props_and_attrs(const struct igt_device *dev)
> +{
> +	printf("\n[properties]\n");
> +	print_ht(dev->props_ht);
> +	printf("\n[attributes]\n");
> +	print_ht(dev->attrs_ht);
> +	printf("\n");
> +}
> +
>  /*
>   * Get PCI_SLOT_NAME property, it should be in format of
>   * xxxx:yy:zz.z
> @@ -565,9 +575,20 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
>  static void set_pci_slot_name(struct igt_device *dev)
>  {
>  	const char *pci_slot_name = get_prop(dev, "PCI_SLOT_NAME");
> +	int len;
> +
> +	if (!pci_slot_name) {
> +		dump_props_and_attrs(dev);
> +		igt_assert_f(pci_slot_name, "PCI_SLOT_NAME property == NULL\n");
> +	}
> +
> +	len = strlen(pci_slot_name);
> +	if (len != PCI_SLOT_NAME_SIZE) {
> +		dump_props_and_attrs(dev);
> +		igt_assert_f(len != PCI_SLOT_NAME_SIZE,
> +			     "PCI_SLOT_NAME length != %d [%s]\n", len, pci_slot_name);
> +	}
>  
> -	if (!pci_slot_name || (strlen(pci_slot_name) != PCI_SLOT_NAME_SIZE))
> -		return;
>  	dev->pci_slot_name = strdup(pci_slot_name);
>  }
>  
> @@ -1249,11 +1270,7 @@ igt_devs_print_detail(struct igt_list_head *view,
>  			_print_key_value("codename", dev->codename);
>  		}
>  
> -		printf("\n[properties]\n");
> -		print_ht(dev->props_ht);
> -		printf("\n[attributes]\n");
> -		print_ht(dev->attrs_ht);
> -		printf("\n");
> +		dump_props_and_attrs(dev);
>  	}
>  }
>  
> -- 
> 2.34.1
>