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 <purvayeshi550@gmail.com>
---
 drivers/firewire/ohci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index edaedd156a6d..c05d90511f2b 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -1392,7 +1392,7 @@ static int at_context_queue_packet(struct context *ctx,
 				   struct fw_packet *packet)
 {
 	struct fw_ohci *ohci = ctx->ohci;
-	dma_addr_t d_bus, payload_bus;
+	dma_addr_t d_bus, payload_bus = 0;
 	struct driver_data *driver_data;
 	struct descriptor *d, *last;
 	__le32 *header;
-- 
2.34.1

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 <purvayeshi550@gmail.com>

In my opinion, we should pay enough attention to the value of
'packet->payload_mapped' variable when considering the issue.

```
$ 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

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 <purvayeshi550@gmail.com>
> 
> 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