linux-kernel - Re: [PATCH RESEND v2 5/7] PCI/sysfs: Allow userspace to query and set device reset mechanism

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210520163737.GD641812@rocinante.localdomain>
Date:   Thu, 20 May 2021 18:37:37 +0200
From:   Krzysztof Wilczyński <kw@...ux.com>
To:     Amey Narkhede <ameynarkhede03@...il.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>, alex.williamson@...hat.com,
        raphael.norwitz@...anix.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND v2 5/7] PCI/sysfs: Allow userspace to query and
 set device reset mechanism

Hi Amey,

[...]
> +	if (sysfs_streq(buf, "")) {
> +		pci_warn(pdev, "All device reset methods disabled by user");
> +		goto set_reset_methods;
> +	}

The sysfs_streq() is nice, indeed.

> +	while ((name = strsep((char **)&buf, ",")) != NULL) {

I believe we could make this parsing a little bit more resilient,
especially since we are handling user input and this cannot ever be
really fully trusted, so for example:

  while ((name = strsep((char **)&buf, ","))) {
	if !(strlen(name))  <--- sysfs_streq() could be used here too.
		continue;

	name = strim(name); <--- remove leading and trailing whitespaces, if any.
	(...)

[...]
> +	if (reset_methods[0] &&
> +	    reset_methods[0] != PCI_RESET_FN_METHODS)
> +		pci_warn(pdev, "Device specific reset disabled/de-prioritized by user");

What would be difference between disabling and de-prioritizing, is there
be a way for us to distinguish between the two?  I was wondering if we
could, notify the user when the device specific reset is disable or when
it has been de-prioritized?

Krzysztof