Re: [bug report] Bluetooth: btrtl: split the device initialization into smaller parts

[Dan Carpenter <dan.carpenter@linaro.org>]

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