Re: [PATCH] firewire: ohci: Initialize payload_bus to avoid uninitialized use warning

On 10/07/25 18:22, Takashi Sakamoto wrote:
> Hi,
> 
> On Thu, Jul 10, 2025 at 01:09:06PM +0530, Purva Yeshi wrote:
>> Fix Smatch-detected error:
>> drivers/firewire/ohci.c:1514 at_context_queue_packet()
>> error: uninitialized symbol 'payload_bus'.
>>
>> Smatch reports a potential uninitialized use of 'payload_bus' in
>> at_context_queue_packet(). If packet->payload_length is zero, the
>> variable may not be set before reaching the dma_unmap_single() call,
>> which could lead to undefined behavior.
>>
>> Initialize 'payload_bus' to 0 to ensure it has a defined value in all
>> code paths, preventing any uninitialized access.
>>
>> Signed-off-by: Purva Yeshi <[email protected]>
> 
> In my opinion, we should pay enough attention to the value of
> 'packet->payload_mapped' variable when considering the issue.

Hi Takashi,

Thank you for your review.

The change was mainly to address the Smatch warning, which reports a 
possible uninitialized use. While it might not be a real issue in practice.

> 
> ```
> $ cat -n drivers/firewire/ohci.c
>       ...
> 1385 static int at_context_queue_packet(struct context *ctx,
> 1386                                    struct fw_packet *packet)
> 1387 {
> 1388         struct fw_ohci *ohci = ctx->ohci;
> 1389         dma_addr_t d_bus, payload_bus;
>       ...
> 1474         if (packet->payload_length > 0) {
> 1475                 if (packet->payload_length > sizeof(driver_data->inline_data)) {
> 1476                         payload_bus = dma_map_single(ohci->card.device,
>                               ...
> 1485                         packet->payload_mapped  = true;
> 1486                 } else {
>                               ...
> 1489                         payload_bus = d_bus + 3 * sizeof(*d);
> 1490                 }
>                       ...
> 1496         } else {
>                      ...
> 1499         }
>               ...
> 1506         if (ohci->generation != packet->generation) {
> 1507                 if (packet->payload_mapped)
> 1508                         dma_unmap_single(ohci->card.device, payload_bus,
> 1509                                          packet->payload_length, DMA_TO_DEVICE);
>                       ...
> 1512         }
> 
> Unless the variable has true, the dma_unmap_single() is never called,
> thus the issue does not occur.
> 
> 
> Thanks
> 
> Takashi Sakamoto

Let me know if you would prefer addressing this with a comment 
explaining the assumption, or if you believe it's appropriate to silence 
the Smatch warning in another way.
Alternatively, would you recommend simply ignoring the warning in this case?

Best regards,
Purva Yeshi