linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] firmware_loader: Add debug message with checksum for FW file
@ 2023-03-10 14:04 Amadeusz Sławiński
  2023-03-10 17:30 ` Russ Weight
  2023-03-17 14:15 ` Greg Kroah-Hartman
  0 siblings, 2 replies; 4+ messages in thread
From: Amadeusz Sławiński @ 2023-03-10 14:04 UTC (permalink / raw)
  To: Russ Weight, Luis Chamberlain, Greg Kroah-Hartman, linux-kernel
  Cc: Cezary Rojewski, Rafael J . Wysocki, Amadeusz Sławiński

Enable dynamic-debug logging of firmware filenames and SHA256 checksums
to clearly identify the firmware files that are loaded by the system.

Example output:
[   34.944619] firmware_class:_request_firmware: i915 0000:00:02.0: Loaded FW: i915/kbl_dmc_ver1_04.bin, sha256: 2cde41c3e5ad181423bcc3e98ff9c49f743c88f18646af4d0b3c3a9664b831a1
[   48.155884] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/cnl/dsp_basefw.bin, sha256: 43f6ac1b066e9bd0423d914960fbbdccb391af27d2b1da1085eee3ea8df0f357
[   49.579540] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/rt274-tplg.bin, sha256: 4b3580da96dc3d2c443ba20c6728d8b665fceb3ed57223c3a57582bbad8e2413
[   49.798196] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/hda-8086280c-tplg.bin, sha256: 5653172579b2be1b51fd69f5cf46e2bac8d63f2a1327924311c13b2f1fe6e601
[   49.859627] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/dmic-tplg.bin, sha256: 00fb7fbdb74683333400d7e46925dae60db448b88638efcca0b30215db9df63f

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---

Changes in v4:
 * update menuconfig prompt and help message (Russ)

Changes in v3:
 * add DYNAMIC_DEBUG and FW_LOADER as dependencies before option can be
enabled (kernel test robot)

Changes in v2:
 * allocate buffers (Greg)
 * introduce CONFIG_ option to allow for CONFIG_CRYPTO and CONFIG_CRYPTO_SHA256
dependencies without introducing circular dependency (Greg)
 * add new line between includes and function name (Cezary)

---
 drivers/base/firmware_loader/Kconfig | 13 ++++++++
 drivers/base/firmware_loader/main.c  | 48 +++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 5166b323a0f8..6520e8c9cb38 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -3,6 +3,7 @@ menu "Firmware loader"
 
 config FW_LOADER
 	tristate "Firmware loading facility" if EXPERT
+	select FW_LOADER_DEBUG if DYNAMIC_DEBUG
 	default y
 	help
 	  This enables the firmware loading facility in the kernel. The kernel
@@ -24,6 +25,18 @@ config FW_LOADER
 	  You also want to be sure to enable this built-in if you are going to
 	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
 
+config FW_LOADER_DEBUG
+	bool "Log filenames and checksums for loaded firmware"
+	depends on DYNAMIC_DEBUG
+	depends on FW_LOADER
+	depends on CRYPTO
+	depends on CRYPTO_SHA256
+	default FW_LOADER
+	help
+	  Select this option to use dynamic debug to log firmware filenames and
+	  SHA256 checksums to the kernel log for each firmware file that is
+	  loaded.
+
 if FW_LOADER
 
 config FW_LOADER_PAGED_BUF
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 017c4cdb219e..b2c292ca95e8 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -791,6 +791,50 @@ static void fw_abort_batch_reqs(struct firmware *fw)
 	mutex_unlock(&fw_lock);
 }
 
+#if defined(CONFIG_FW_LOADER_DEBUG)
+#include <crypto/hash.h>
+#include <crypto/sha2.h>
+
+static void fw_log_firmware_info(const struct firmware *fw, const char *name, struct device *device)
+{
+	struct shash_desc *shash;
+	struct crypto_shash *alg;
+	u8 *sha256buf;
+	char *outbuf;
+
+	alg = crypto_alloc_shash("sha256", 0, 0);
+	if (!alg)
+		return;
+
+	sha256buf = kmalloc(SHA256_DIGEST_SIZE, GFP_KERNEL);
+	outbuf = kmalloc(SHA256_BLOCK_SIZE + 1, GFP_KERNEL);
+	shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(alg), GFP_KERNEL);
+	if (!sha256buf || !outbuf || !shash)
+		goto out_free;
+
+	shash->tfm = alg;
+
+	if (crypto_shash_digest(shash, fw->data, fw->size, sha256buf) < 0)
+		goto out_shash;
+
+	for (int i = 0; i < SHA256_DIGEST_SIZE; i++)
+		sprintf(&outbuf[i * 2], "%02x", sha256buf[i]);
+	outbuf[SHA256_BLOCK_SIZE] = 0;
+	dev_dbg(device, "Loaded FW: %s, sha256: %s\n", name, outbuf);
+
+out_shash:
+	crypto_free_shash(alg);
+out_free:
+	kfree(shash);
+	kfree(outbuf);
+	kfree(sha256buf);
+}
+#else
+static void fw_log_firmware_info(const struct firmware *fw, const char *name,
+				 struct device *device)
+{}
+#endif
+
 /* called from request_firmware() and request_firmware_work_func() */
 static int
 _request_firmware(const struct firmware **firmware_p, const char *name,
@@ -861,11 +905,13 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 	revert_creds(old_cred);
 	put_cred(kern_cred);
 
- out:
+out:
 	if (ret < 0) {
 		fw_abort_batch_reqs(fw);
 		release_firmware(fw);
 		fw = NULL;
+	} else {
+		fw_log_firmware_info(fw, name, device);
 	}
 
 	*firmware_p = fw;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] firmware_loader: Add debug message with checksum for FW file
  2023-03-10 14:04 [PATCH v4] firmware_loader: Add debug message with checksum for FW file Amadeusz Sławiński
@ 2023-03-10 17:30 ` Russ Weight
  2023-03-17 14:15 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 4+ messages in thread
From: Russ Weight @ 2023-03-10 17:30 UTC (permalink / raw)
  To: Amadeusz Sławiński, Luis Chamberlain,
	Greg Kroah-Hartman, linux-kernel
  Cc: Cezary Rojewski, Rafael J . Wysocki



On 3/10/23 06:04, Amadeusz Sławiński wrote:
> Enable dynamic-debug logging of firmware filenames and SHA256 checksums
> to clearly identify the firmware files that are loaded by the system.
>
> Example output:
> [   34.944619] firmware_class:_request_firmware: i915 0000:00:02.0: Loaded FW: i915/kbl_dmc_ver1_04.bin, sha256: 2cde41c3e5ad181423bcc3e98ff9c49f743c88f18646af4d0b3c3a9664b831a1
> [   48.155884] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/cnl/dsp_basefw.bin, sha256: 43f6ac1b066e9bd0423d914960fbbdccb391af27d2b1da1085eee3ea8df0f357
> [   49.579540] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/rt274-tplg.bin, sha256: 4b3580da96dc3d2c443ba20c6728d8b665fceb3ed57223c3a57582bbad8e2413
> [   49.798196] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/hda-8086280c-tplg.bin, sha256: 5653172579b2be1b51fd69f5cf46e2bac8d63f2a1327924311c13b2f1fe6e601
> [   49.859627] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/dmic-tplg.bin, sha256: 00fb7fbdb74683333400d7e46925dae60db448b88638efcca0b30215db9df63f
>
> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>

Reviewed-by: Russ Weight <russell.h.weight@intel.com>
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> ---
>
> Changes in v4:
>  * update menuconfig prompt and help message (Russ)
>
> Changes in v3:
>  * add DYNAMIC_DEBUG and FW_LOADER as dependencies before option can be
> enabled (kernel test robot)
>
> Changes in v2:
>  * allocate buffers (Greg)
>  * introduce CONFIG_ option to allow for CONFIG_CRYPTO and CONFIG_CRYPTO_SHA256
> dependencies without introducing circular dependency (Greg)
>  * add new line between includes and function name (Cezary)
>
> ---
>  drivers/base/firmware_loader/Kconfig | 13 ++++++++
>  drivers/base/firmware_loader/main.c  | 48 +++++++++++++++++++++++++++-
>  2 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
> index 5166b323a0f8..6520e8c9cb38 100644
> --- a/drivers/base/firmware_loader/Kconfig
> +++ b/drivers/base/firmware_loader/Kconfig
> @@ -3,6 +3,7 @@ menu "Firmware loader"
>  
>  config FW_LOADER
>  	tristate "Firmware loading facility" if EXPERT
> +	select FW_LOADER_DEBUG if DYNAMIC_DEBUG
>  	default y
>  	help
>  	  This enables the firmware loading facility in the kernel. The kernel
> @@ -24,6 +25,18 @@ config FW_LOADER
>  	  You also want to be sure to enable this built-in if you are going to
>  	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
>  
> +config FW_LOADER_DEBUG
> +	bool "Log filenames and checksums for loaded firmware"
> +	depends on DYNAMIC_DEBUG
> +	depends on FW_LOADER
> +	depends on CRYPTO
> +	depends on CRYPTO_SHA256
> +	default FW_LOADER
> +	help
> +	  Select this option to use dynamic debug to log firmware filenames and
> +	  SHA256 checksums to the kernel log for each firmware file that is
> +	  loaded.
> +
>  if FW_LOADER
>  
>  config FW_LOADER_PAGED_BUF
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index 017c4cdb219e..b2c292ca95e8 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -791,6 +791,50 @@ static void fw_abort_batch_reqs(struct firmware *fw)
>  	mutex_unlock(&fw_lock);
>  }
>  
> +#if defined(CONFIG_FW_LOADER_DEBUG)
> +#include <crypto/hash.h>
> +#include <crypto/sha2.h>
> +
> +static void fw_log_firmware_info(const struct firmware *fw, const char *name, struct device *device)
> +{
> +	struct shash_desc *shash;
> +	struct crypto_shash *alg;
> +	u8 *sha256buf;
> +	char *outbuf;
> +
> +	alg = crypto_alloc_shash("sha256", 0, 0);
> +	if (!alg)
> +		return;
> +
> +	sha256buf = kmalloc(SHA256_DIGEST_SIZE, GFP_KERNEL);
> +	outbuf = kmalloc(SHA256_BLOCK_SIZE + 1, GFP_KERNEL);
> +	shash = kmalloc(sizeof(*shash) + crypto_shash_descsize(alg), GFP_KERNEL);
> +	if (!sha256buf || !outbuf || !shash)
> +		goto out_free;
> +
> +	shash->tfm = alg;
> +
> +	if (crypto_shash_digest(shash, fw->data, fw->size, sha256buf) < 0)
> +		goto out_shash;
> +
> +	for (int i = 0; i < SHA256_DIGEST_SIZE; i++)
> +		sprintf(&outbuf[i * 2], "%02x", sha256buf[i]);
> +	outbuf[SHA256_BLOCK_SIZE] = 0;
> +	dev_dbg(device, "Loaded FW: %s, sha256: %s\n", name, outbuf);
> +
> +out_shash:
> +	crypto_free_shash(alg);
> +out_free:
> +	kfree(shash);
> +	kfree(outbuf);
> +	kfree(sha256buf);
> +}
> +#else
> +static void fw_log_firmware_info(const struct firmware *fw, const char *name,
> +				 struct device *device)
> +{}
> +#endif
> +
>  /* called from request_firmware() and request_firmware_work_func() */
>  static int
>  _request_firmware(const struct firmware **firmware_p, const char *name,
> @@ -861,11 +905,13 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
>  	revert_creds(old_cred);
>  	put_cred(kern_cred);
>  
> - out:
> +out:
>  	if (ret < 0) {
>  		fw_abort_batch_reqs(fw);
>  		release_firmware(fw);
>  		fw = NULL;
> +	} else {
> +		fw_log_firmware_info(fw, name, device);
>  	}
>  
>  	*firmware_p = fw;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] firmware_loader: Add debug message with checksum for FW file
  2023-03-10 14:04 [PATCH v4] firmware_loader: Add debug message with checksum for FW file Amadeusz Sławiński
  2023-03-10 17:30 ` Russ Weight
@ 2023-03-17 14:15 ` Greg Kroah-Hartman
  2023-03-17 14:46   ` Amadeusz Sławiński
  1 sibling, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2023-03-17 14:15 UTC (permalink / raw)
  To: Amadeusz Sławiński
  Cc: Russ Weight, Luis Chamberlain, linux-kernel, Cezary Rojewski,
	Rafael J . Wysocki

On Fri, Mar 10, 2023 at 03:04:59PM +0100, Amadeusz Sławiński wrote:
> Enable dynamic-debug logging of firmware filenames and SHA256 checksums
> to clearly identify the firmware files that are loaded by the system.
> 
> Example output:
> [   34.944619] firmware_class:_request_firmware: i915 0000:00:02.0: Loaded FW: i915/kbl_dmc_ver1_04.bin, sha256: 2cde41c3e5ad181423bcc3e98ff9c49f743c88f18646af4d0b3c3a9664b831a1
> [   48.155884] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/cnl/dsp_basefw.bin, sha256: 43f6ac1b066e9bd0423d914960fbbdccb391af27d2b1da1085eee3ea8df0f357
> [   49.579540] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/rt274-tplg.bin, sha256: 4b3580da96dc3d2c443ba20c6728d8b665fceb3ed57223c3a57582bbad8e2413
> [   49.798196] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/hda-8086280c-tplg.bin, sha256: 5653172579b2be1b51fd69f5cf46e2bac8d63f2a1327924311c13b2f1fe6e601
> [   49.859627] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/dmic-tplg.bin, sha256: 00fb7fbdb74683333400d7e46925dae60db448b88638efcca0b30215db9df63f
> 
> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> ---
> 
> Changes in v4:
>  * update menuconfig prompt and help message (Russ)
> 
> Changes in v3:
>  * add DYNAMIC_DEBUG and FW_LOADER as dependencies before option can be
> enabled (kernel test robot)
> 
> Changes in v2:
>  * allocate buffers (Greg)
>  * introduce CONFIG_ option to allow for CONFIG_CRYPTO and CONFIG_CRYPTO_SHA256
> dependencies without introducing circular dependency (Greg)
>  * add new line between includes and function name (Cezary)
> 
> ---
>  drivers/base/firmware_loader/Kconfig | 13 ++++++++
>  drivers/base/firmware_loader/main.c  | 48 +++++++++++++++++++++++++++-
>  2 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
> index 5166b323a0f8..6520e8c9cb38 100644
> --- a/drivers/base/firmware_loader/Kconfig
> +++ b/drivers/base/firmware_loader/Kconfig
> @@ -3,6 +3,7 @@ menu "Firmware loader"
>  
>  config FW_LOADER
>  	tristate "Firmware loading facility" if EXPERT
> +	select FW_LOADER_DEBUG if DYNAMIC_DEBUG

Why the select?  that prevents anyone from actually choosing this if
they want to or not.  It also prevents them from disabling this option
if they want to, while still keeping DYNAMIC_DEBUG enabled.

So please don't make this change.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] firmware_loader: Add debug message with checksum for FW file
  2023-03-17 14:15 ` Greg Kroah-Hartman
@ 2023-03-17 14:46   ` Amadeusz Sławiński
  0 siblings, 0 replies; 4+ messages in thread
From: Amadeusz Sławiński @ 2023-03-17 14:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Russ Weight, Luis Chamberlain, linux-kernel, Cezary Rojewski,
	Rafael J . Wysocki

On 3/17/2023 3:15 PM, Greg Kroah-Hartman wrote:
> On Fri, Mar 10, 2023 at 03:04:59PM +0100, Amadeusz Sławiński wrote:
>> Enable dynamic-debug logging of firmware filenames and SHA256 checksums
>> to clearly identify the firmware files that are loaded by the system.
>>
>> Example output:
>> [   34.944619] firmware_class:_request_firmware: i915 0000:00:02.0: Loaded FW: i915/kbl_dmc_ver1_04.bin, sha256: 2cde41c3e5ad181423bcc3e98ff9c49f743c88f18646af4d0b3c3a9664b831a1
>> [   48.155884] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/cnl/dsp_basefw.bin, sha256: 43f6ac1b066e9bd0423d914960fbbdccb391af27d2b1da1085eee3ea8df0f357
>> [   49.579540] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/rt274-tplg.bin, sha256: 4b3580da96dc3d2c443ba20c6728d8b665fceb3ed57223c3a57582bbad8e2413
>> [   49.798196] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/hda-8086280c-tplg.bin, sha256: 5653172579b2be1b51fd69f5cf46e2bac8d63f2a1327924311c13b2f1fe6e601
>> [   49.859627] firmware_class:_request_firmware: snd_soc_avs 0000:00:1f.3: Loaded FW: intel/avs/dmic-tplg.bin, sha256: 00fb7fbdb74683333400d7e46925dae60db448b88638efcca0b30215db9df63f
>>
>> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>> ---
>>
>> Changes in v4:
>>   * update menuconfig prompt and help message (Russ)
>>
>> Changes in v3:
>>   * add DYNAMIC_DEBUG and FW_LOADER as dependencies before option can be
>> enabled (kernel test robot)
>>
>> Changes in v2:
>>   * allocate buffers (Greg)
>>   * introduce CONFIG_ option to allow for CONFIG_CRYPTO and CONFIG_CRYPTO_SHA256
>> dependencies without introducing circular dependency (Greg)
>>   * add new line between includes and function name (Cezary)
>>
>> ---
>>   drivers/base/firmware_loader/Kconfig | 13 ++++++++
>>   drivers/base/firmware_loader/main.c  | 48 +++++++++++++++++++++++++++-
>>   2 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
>> index 5166b323a0f8..6520e8c9cb38 100644
>> --- a/drivers/base/firmware_loader/Kconfig
>> +++ b/drivers/base/firmware_loader/Kconfig
>> @@ -3,6 +3,7 @@ menu "Firmware loader"
>>   
>>   config FW_LOADER
>>   	tristate "Firmware loading facility" if EXPERT
>> +	select FW_LOADER_DEBUG if DYNAMIC_DEBUG
> 
> Why the select?  that prevents anyone from actually choosing this if
> they want to or not.  It also prevents them from disabling this option
> if they want to, while still keeping DYNAMIC_DEBUG enabled.
> 
> So please don't make this change.
>
Indeed it seems unnecessary, I removed it in v5.

Thanks,

Amadeusz


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-03-17 14:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-10 14:04 [PATCH v4] firmware_loader: Add debug message with checksum for FW file Amadeusz Sławiński
2023-03-10 17:30 ` Russ Weight
2023-03-17 14:15 ` Greg Kroah-Hartman
2023-03-17 14:46   ` Amadeusz Sławiński

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).