diff mbox series

[net-next,v2] firmware_loader: Expand Firmware upload error codes with firmware invalid error

Message ID 20231121-feature_firmware_error_code-v2-1-f879a7734a4e@bootlin.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v2] firmware_loader: Expand Firmware upload error codes with firmware invalid error | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/codegen success Generated files up to date
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3523 this patch: 3523
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/build_clang fail Errors and warnings before: 1364 this patch: 1365
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 3900 this patch: 3900
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 21 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kory Maincent Nov. 21, 2023, 10:50 a.m. UTC 
No error code are available to signal an invalid firmware content.
Drivers that can check the firmware content validity can not return this
specific failure to the user-space

Expand the firmware error code with an additional code:
- "firmware invalid" code which can be used when the provided firmware
  is invalid

Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

This patch was initially submitted as part of a net patch series.
Conor expressed interest in using it in a different subsystem.
https://lore.kernel.org/netdev/20231116-feature_poe-v1-7-be48044bf249@bootlin.com/

Consequently, I extracted it from the series and submitted it separately.
I first tried to send it to driver-core but it seems also not the best
choice:
https://lore.kernel.org/lkml/2023111720-slicer-exes-7d9f@gregkh/

Jakub could you create a stable branch for this patch and share the branch
information? This way other Maintainers can then pull the patch.
---
 drivers/base/firmware_loader/sysfs_upload.c | 1 +
 include/linux/firmware.h                    | 2 ++
 2 files changed, 3 insertions(+)


---
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
change-id: 20231117-feature_firmware_error_code-b8d7af08a8fe

Best regards,

Comments

Jakub Kicinski Nov. 22, 2023, 1:30 a.m. UTC | #1 
On Tue, 21 Nov 2023 11:50:35 +0100 Kory Maincent wrote:
> No error code are available to signal an invalid firmware content.
> Drivers that can check the firmware content validity can not return this
> specific failure to the user-space
> 
> Expand the firmware error code with an additional code:
> - "firmware invalid" code which can be used when the provided firmware
>   is invalid

Any idea what this is?

lib/test_firmware.o: warning: objtool: test_fw_upload_prepare() falls through to next function __cfi_test_fw_upload_cancel()

My build shows this on an incremental clang 17 build.
Kory Maincent Nov. 22, 2023, 9:16 a.m. UTC | #2 
Hello Jakub,

On Tue, 21 Nov 2023 17:30:22 -0800
Jakub Kicinski <kuba@kernel.org> wrote:

> On Tue, 21 Nov 2023 11:50:35 +0100 Kory Maincent wrote:
> > No error code are available to signal an invalid firmware content.
> > Drivers that can check the firmware content validity can not return this
> > specific failure to the user-space
> > 
> > Expand the firmware error code with an additional code:
> > - "firmware invalid" code which can be used when the provided firmware
> >   is invalid  
> 
> Any idea what this is?
> 
> lib/test_firmware.o: warning: objtool: test_fw_upload_prepare() falls through
> to next function __cfi_test_fw_upload_cancel()
> 
> My build shows this on an incremental clang 17 build.

Thanks for the report.
It seems I have to update fw_upload_err_str accordingly.
Didn't know about this test_firmware.c file. 

Regards,
Kory Maincent Nov. 22, 2023, 10:43 a.m. UTC | #3 
On Tue, 21 Nov 2023 17:30:22 -0800
Jakub Kicinski <kuba@kernel.org> wrote:

> On Tue, 21 Nov 2023 11:50:35 +0100 Kory Maincent wrote:
> > No error code are available to signal an invalid firmware content.
> > Drivers that can check the firmware content validity can not return this
> > specific failure to the user-space
> > 
> > Expand the firmware error code with an additional code:
> > - "firmware invalid" code which can be used when the provided firmware
> >   is invalid  
> 
> Any idea what this is?
> 
> lib/test_firmware.o: warning: objtool: test_fw_upload_prepare() falls through
> to next function __cfi_test_fw_upload_cancel()
> 
> My build shows this on an incremental clang 17 build.

For my curiosity, how do you get this error?

Enabling test_firmware and building with W=1 does not show the error.
 
Regards,
Simon Horman Nov. 24, 2023, 4:44 p.m. UTC | #4 
On Wed, Nov 22, 2023 at 11:43:25AM +0100, Köry Maincent wrote:
> On Tue, 21 Nov 2023 17:30:22 -0800
> Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > On Tue, 21 Nov 2023 11:50:35 +0100 Kory Maincent wrote:
> > > No error code are available to signal an invalid firmware content.
> > > Drivers that can check the firmware content validity can not return this
> > > specific failure to the user-space
> > > 
> > > Expand the firmware error code with an additional code:
> > > - "firmware invalid" code which can be used when the provided firmware
> > >   is invalid  
> > 
> > Any idea what this is?
> > 
> > lib/test_firmware.o: warning: objtool: test_fw_upload_prepare() falls through
> > to next function __cfi_test_fw_upload_cancel()
> > 
> > My build shows this on an incremental clang 17 build.
> 
> For my curiosity, how do you get this error?
> 
> Enabling test_firmware and building with W=1 does not show the error.

Hi Kory,

I am able to observe this warning when compiling with clang-16

make LLVM=1 lib/test_firmware.o

...
diff mbox series

Patch

diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index a0af8f5f13d8..829270067d16 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -27,6 +27,7 @@  static const char * const fw_upload_err_str[] = {
 	[FW_UPLOAD_ERR_INVALID_SIZE] = "invalid-file-size",
 	[FW_UPLOAD_ERR_RW_ERROR]     = "read-write-error",
 	[FW_UPLOAD_ERR_WEAROUT]	     = "flash-wearout",
+	[FW_UPLOAD_ERR_FW_INVALID]   = "firmware-invalid",
 };
 
 static const char *fw_upload_progress(struct device *dev,
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index de7fea3bca51..0311858b46ce 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -27,6 +27,7 @@  struct firmware {
  * @FW_UPLOAD_ERR_INVALID_SIZE: invalid firmware image size
  * @FW_UPLOAD_ERR_RW_ERROR: read or write to HW failed, see kernel log
  * @FW_UPLOAD_ERR_WEAROUT: FLASH device is approaching wear-out, wait & retry
+ * @FW_UPLOAD_ERR_FW_INVALID: invalid firmware file
  * @FW_UPLOAD_ERR_MAX: Maximum error code marker
  */
 enum fw_upload_err {
@@ -38,6 +39,7 @@  enum fw_upload_err {
 	FW_UPLOAD_ERR_INVALID_SIZE,
 	FW_UPLOAD_ERR_RW_ERROR,
 	FW_UPLOAD_ERR_WEAROUT,
+	FW_UPLOAD_ERR_FW_INVALID,
 	FW_UPLOAD_ERR_MAX
 };