* [bug report] Bluetooth: btrtl: split the device initialization into smaller parts
@ 2018-08-06 20:42 Dan Carpenter
2025-03-21 14:35 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2018-08-06 20:42 UTC (permalink / raw)
To: martin.blumenstingl; +Cc: linux-bluetooth
Hello Martin Blumenstingl,
The patch 26503ad25de8: "Bluetooth: btrtl: split the device
initialization into smaller parts" from Aug 2, 2018, leads to the
following static checker warning:
drivers/bluetooth/btrtl.c:592 btrtl_initialize()
warn: passing zero to 'ERR_PTR'
drivers/bluetooth/btrtl.c
559 btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
560 &btrtl_dev->fw_data);
561 if (btrtl_dev->fw_len < 0) {
562 rtl_dev_err(hdev, "firmware file %s not found\n",
563 btrtl_dev->ic_info->fw_name);
564 ret = btrtl_dev->fw_len;
565 goto err_free;
566 }
567
568 if (btrtl_dev->ic_info->cfg_name) {
569 if (postfix) {
570 snprintf(cfg_name, sizeof(cfg_name), "%s-%s.bin",
571 btrtl_dev->ic_info->cfg_name, postfix);
572 } else {
573 snprintf(cfg_name, sizeof(cfg_name), "%s.bin",
574 btrtl_dev->ic_info->cfg_name);
575 }
576 btrtl_dev->cfg_len = rtl_load_file(hdev, cfg_name,
577 &btrtl_dev->cfg_data);
578 if (btrtl_dev->ic_info->config_needed &&
579 btrtl_dev->cfg_len <= 0) {
^^^^^^^^^^^^^^^^^^^^^^^
Assume btrtl_dev->cfg_len == 0
580 rtl_dev_err(hdev, "mandatory config file %s not found\n",
581 btrtl_dev->ic_info->cfg_name);
582 ret = btrtl_dev->cfg_len;
583 goto err_free;
584 }
585 }
586
587 return btrtl_dev;
588
589 err_free:
590 btrtl_free(btrtl_dev);
591 err_alloc:
592 return ERR_PTR(ret);
^^^
Then we would end up returning ERR_PTR(0) which is NULL and would result
in a NULL dereference in the error pointer.
593 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] Bluetooth: btrtl: split the device initialization into smaller parts
2018-08-06 20:42 [bug report] Bluetooth: btrtl: split the device initialization into smaller parts Dan Carpenter
@ 2025-03-21 14:35 ` Dan Carpenter
2025-03-29 21:53 ` Luis Chamberlain
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2025-03-21 14:35 UTC (permalink / raw)
To: Luis Chamberlain, Russ Weight, Danilo Krummrich
Cc: martin.blumenstingl, linux-bluetooth
On Mon, Aug 06, 2018 at 11:42:57PM +0300, Dan Carpenter wrote:
> Hello Martin Blumenstingl,
>
> The patch 26503ad25de8: "Bluetooth: btrtl: split the device
> initialization into smaller parts" from Aug 2, 2018, leads to the
> following static checker warning:
>
> drivers/bluetooth/btrtl.c:592 btrtl_initialize()
> warn: passing zero to 'ERR_PTR'
>
> drivers/bluetooth/btrtl.c
> 559 btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
> 560 &btrtl_dev->fw_data);
> 561 if (btrtl_dev->fw_len < 0) {
> 562 rtl_dev_err(hdev, "firmware file %s not found\n",
> 563 btrtl_dev->ic_info->fw_name);
> 564 ret = btrtl_dev->fw_len;
> 565 goto err_free;
> 566 }
> 567
> 568 if (btrtl_dev->ic_info->cfg_name) {
> 569 if (postfix) {
> 570 snprintf(cfg_name, sizeof(cfg_name), "%s-%s.bin",
> 571 btrtl_dev->ic_info->cfg_name, postfix);
> 572 } else {
> 573 snprintf(cfg_name, sizeof(cfg_name), "%s.bin",
> 574 btrtl_dev->ic_info->cfg_name);
> 575 }
> 576 btrtl_dev->cfg_len = rtl_load_file(hdev, cfg_name,
> 577 &btrtl_dev->cfg_data);
> 578 if (btrtl_dev->ic_info->config_needed &&
> 579 btrtl_dev->cfg_len <= 0) {
> ^^^^^^^^^^^^^^^^^^^^^^^
> Assume btrtl_dev->cfg_len == 0
>
This is the length of the firmware file. Does it make sense for
request_firmware() to load empty files? Probably there is a test for
this in the firmware code which rejects zero length files?
regards,
dan carpenter
> 580 rtl_dev_err(hdev, "mandatory config file %s not found\n",
> 581 btrtl_dev->ic_info->cfg_name);
> 582 ret = btrtl_dev->cfg_len;
> 583 goto err_free;
> 584 }
> 585 }
> 586
> 587 return btrtl_dev;
> 588
> 589 err_free:
> 590 btrtl_free(btrtl_dev);
> 591 err_alloc:
> 592 return ERR_PTR(ret);
> ^^^
> Then we would end up returning ERR_PTR(0) which is NULL and would result
> in a NULL dereference in the error pointer.
>
> 593 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] Bluetooth: btrtl: split the device initialization into smaller parts
2025-03-21 14:35 ` Dan Carpenter
@ 2025-03-29 21:53 ` Luis Chamberlain
2025-04-02 11:04 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Luis Chamberlain @ 2025-03-29 21:53 UTC (permalink / raw)
To: Dan Carpenter
Cc: Russ Weight, Danilo Krummrich, martin.blumenstingl,
linux-bluetooth
On Fri, Mar 21, 2025 at 05:35:18PM +0300, Dan Carpenter wrote:
> On Mon, Aug 06, 2018 at 11:42:57PM +0300, Dan Carpenter wrote:
> > Hello Martin Blumenstingl,
> >
> > The patch 26503ad25de8: "Bluetooth: btrtl: split the device
> > initialization into smaller parts" from Aug 2, 2018, leads to the
> > following static checker warning:
> >
> > drivers/bluetooth/btrtl.c:592 btrtl_initialize()
> > warn: passing zero to 'ERR_PTR'
> >
> > drivers/bluetooth/btrtl.c
> > 559 btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
> > 560 &btrtl_dev->fw_data);
> > 561 if (btrtl_dev->fw_len < 0) {
> > 562 rtl_dev_err(hdev, "firmware file %s not found\n",
> > 563 btrtl_dev->ic_info->fw_name);
> > 564 ret = btrtl_dev->fw_len;
> > 565 goto err_free;
> > 566 }
> > 567
> > 568 if (btrtl_dev->ic_info->cfg_name) {
> > 569 if (postfix) {
> > 570 snprintf(cfg_name, sizeof(cfg_name), "%s-%s.bin",
> > 571 btrtl_dev->ic_info->cfg_name, postfix);
> > 572 } else {
> > 573 snprintf(cfg_name, sizeof(cfg_name), "%s.bin",
> > 574 btrtl_dev->ic_info->cfg_name);
> > 575 }
> > 576 btrtl_dev->cfg_len = rtl_load_file(hdev, cfg_name,
> > 577 &btrtl_dev->cfg_data);
> > 578 if (btrtl_dev->ic_info->config_needed &&
> > 579 btrtl_dev->cfg_len <= 0) {
> > ^^^^^^^^^^^^^^^^^^^^^^^
> > Assume btrtl_dev->cfg_len == 0
> >
>
> This is the length of the firmware file. Does it make sense for
> request_firmware() to load empty files? Probably there is a test for
> this in the firmware code which rejects zero length files?
We don't know the size of the file until we try to read it. Although
kernel_read_file_from_path_initns() perhaps should allow for empty
files, I do agree it seems odd to use the firmware API for 0 length
files.
We should extend tools/testing/selftests/firmware/ to check for this.
Care for a patch?
Luis
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] Bluetooth: btrtl: split the device initialization into smaller parts
2025-03-29 21:53 ` Luis Chamberlain
@ 2025-04-02 11:04 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2025-04-02 11:04 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Russ Weight, Danilo Krummrich, martin.blumenstingl,
linux-bluetooth
On Sat, Mar 29, 2025 at 02:53:22PM -0700, Luis Chamberlain wrote:
> On Fri, Mar 21, 2025 at 05:35:18PM +0300, Dan Carpenter wrote:
> > On Mon, Aug 06, 2018 at 11:42:57PM +0300, Dan Carpenter wrote:
> > > Hello Martin Blumenstingl,
> > >
> > > The patch 26503ad25de8: "Bluetooth: btrtl: split the device
> > > initialization into smaller parts" from Aug 2, 2018, leads to the
> > > following static checker warning:
> > >
> > > drivers/bluetooth/btrtl.c:592 btrtl_initialize()
> > > warn: passing zero to 'ERR_PTR'
> > >
> > > drivers/bluetooth/btrtl.c
> > > 559 btrtl_dev->fw_len = rtl_load_file(hdev, btrtl_dev->ic_info->fw_name,
> > > 560 &btrtl_dev->fw_data);
> > > 561 if (btrtl_dev->fw_len < 0) {
> > > 562 rtl_dev_err(hdev, "firmware file %s not found\n",
> > > 563 btrtl_dev->ic_info->fw_name);
> > > 564 ret = btrtl_dev->fw_len;
> > > 565 goto err_free;
> > > 566 }
> > > 567
> > > 568 if (btrtl_dev->ic_info->cfg_name) {
> > > 569 if (postfix) {
> > > 570 snprintf(cfg_name, sizeof(cfg_name), "%s-%s.bin",
> > > 571 btrtl_dev->ic_info->cfg_name, postfix);
> > > 572 } else {
> > > 573 snprintf(cfg_name, sizeof(cfg_name), "%s.bin",
> > > 574 btrtl_dev->ic_info->cfg_name);
> > > 575 }
> > > 576 btrtl_dev->cfg_len = rtl_load_file(hdev, cfg_name,
> > > 577 &btrtl_dev->cfg_data);
> > > 578 if (btrtl_dev->ic_info->config_needed &&
> > > 579 btrtl_dev->cfg_len <= 0) {
> > > ^^^^^^^^^^^^^^^^^^^^^^^
> > > Assume btrtl_dev->cfg_len == 0
> > >
> >
> > This is the length of the firmware file. Does it make sense for
> > request_firmware() to load empty files? Probably there is a test for
> > this in the firmware code which rejects zero length files?
>
> We don't know the size of the file until we try to read it. Although
> kernel_read_file_from_path_initns() perhaps should allow for empty
> files, I do agree it seems odd to use the firmware API for 0 length
> files.
>
> We should extend tools/testing/selftests/firmware/ to check for this.
> Care for a patch?
I was hoping that maybe the code already existed and I just hadn't seen
it. I wouldn't have the foggiest idea how to write it myself. This code
returns 0 if there is a zero length file and that results in a NULL
dereference. I've sent a fix for that.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-04-02 11:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-06 20:42 [bug report] Bluetooth: btrtl: split the device initialization into smaller parts Dan Carpenter
2025-03-21 14:35 ` Dan Carpenter
2025-03-29 21:53 ` Luis Chamberlain
2025-04-02 11:04 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox