Re: [bug report] firewire: cdev: obsolete NULL check to detect IEC 61883-1 FCP region

Re: [bug report] firewire: cdev: obsolete NULL check to detect IEC 61883-1 FCP region

[Takashi Sakamoto]

Hi,
(C.C.ed to LKML and alsa-devel)

On Mon, Feb 27, 2023 at 02:06:51PM +0300, Dan Carpenter wrote:
> Hello Takashi Sakamoto,
> 
> The patch e699600232e0: "firewire: cdev: obsolete NULL check to
> detect IEC 61883-1 FCP region" from Jan 20, 2023, leads to the
> following Smatch static checker warning:
> 
> 	drivers/firewire/core-transaction.c:947 handle_fcp_region_request()
> 	warn: passing freed memory 'request'
> 
> drivers/firewire/core-transaction.c
>     930                 fw_send_response(card, request, RCODE_TYPE_ERROR);
>     931 
>     932                 return;
>     933         }
>     934 
>     935         rcu_read_lock();
>     936         list_for_each_entry_rcu(handler, &address_handler_list, link) {
>     937                 if (is_enclosing_handler(handler, offset, request->length))
>     938                         handler->address_callback(card, request, tcode,
>                                                                 ^^^^^^^
> This warning is because fwnet_receive_packet() has a kfree(r) on the
> first return path.
> 
>     939                                                   destination, source,
>     940                                                   p->generation, offset,
>     941                                                   request->data,
>     942                                                   request->length,
>     943                                                   handler->callback_data);
>     944         }
>     945         rcu_read_unlock();
>     946 
> --> 947         fw_send_response(card, request, RCODE_COMPLETE);
>     948 }

Thanks for your report.

Fortunately, We can not see the access to the released memory since the
fwnet's address handler is registered to high memory region
(0x'0001'0000'0000 to 0x'ffff'e000'0000). The region does not overlap
IEC 61883-1 FCP region (0x'ffff'f000'0b00 to 0x'ffff'f000'0f00). The
handler is called from handle_exclusive_region_request() instead of
handle_fcp_region_request().

However, the code in fwnet is against the design of address handler
apparently. The callee never release the memory for the request structure
directly. It should be done by the call of fw_send_response(). I'll
correct it for next merge window; i.e. for v6.4.


Thanks

Takashi Sakamoto