Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope

[Jason Wang]

On 11/20/2014 02:29 PM, Paolo Bonzini wrote:
>
> On 20/11/2014 06:57, address@hidden wrote:
>> From: Gonglei <address@hidden>
>>
>> Coverity spot:
>>  Assigning: iov = struct iovec [3]({{buf, 12UL},
>>                        {(void *)dot1q_buf, 4UL},
>>                        {buf + 12, size - 12}})
>>  (address of temporary variable of type struct iovec [3]).
>>  out_of_scope: Temporary variable of type struct iovec [3] goes out of scope.
>>
>> Pointer to local outside scope (RETURN_LOCAL)
>> use_invalid:
>>  Using iov, which points to an out-of-scope temporary variable of type 
>> struct iovec [3].
>>
>> Signed-off-by: Gonglei <address@hidden>
>> ---
>>  hw/net/rtl8139.c | 36 ++++++++++++------------------------
>>  1 file changed, 12 insertions(+), 24 deletions(-)
>>
>> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
>> index 8b8a1b1..426171b 100644
>> --- a/hw/net/rtl8139.c
>> +++ b/hw/net/rtl8139.c
>> @@ -1775,6 +1775,8 @@ static void rtl8139_transfer_frame(RTL8139State *s, 
>> uint8_t *buf, int size,
>>      int do_interrupt, const uint8_t *dot1q_buf)
>>  {
>>      struct iovec *iov = NULL;
>> +    size_t buf2_size;
>> +    uint8_t *buf2 = NULL;
>>  
>>      if (!size)
>>      {
>> @@ -1789,35 +1791,21 @@ static void rtl8139_transfer_frame(RTL8139State *s, 
>> uint8_t *buf, int size,
>>              { .iov_base = buf + ETHER_ADDR_LEN * 2,
>>                  .iov_len = size - ETHER_ADDR_LEN * 2 },
>>          };
>> -    }
>>  
>> -    if (TxLoopBack == (s->TxConfig & TxLoopBack))
>> -    {
>> -        size_t buf2_size;
>> -        uint8_t *buf2;
>> -
>> -        if (iov) {
>> -            buf2_size = iov_size(iov, 3);
>> -            buf2 = g_malloc(buf2_size);
>> -            iov_to_buf(iov, 3, 0, buf2, buf2_size);
>> -            buf = buf2;
>> -        }
>> +        buf2_size = iov_size(iov, 3);
>> +        buf2 = g_malloc(buf2_size);
>> +        iov_to_buf(iov, 3, 0, buf2, buf2_size);
>> +        buf = buf2;
>> +    }
> This makes rtl8139 even slower than it is for the vlantag case, using a
> bounce buffer for every packet.  Perhaps another solution could be to do
>
>     struct iovec *iov = NULL;
>     struct iovec vlan_iov[3];
>
>     ...
>     if (dot1q_buf && size >= ETHER_ADDR_LEN * 2) {
>        ...
>        memcpy(vlan_iov, &(struct iovec[3]) {
>                ...
>            }, sizeof(vlan_iov));
>        iov = vlan_iov;
>     }
>
> (I think "vlan_iov = (struct iovec[3]) { ... };" does not work, but I
> may be wrong).
>
> Stefan, what do you think?
>
> Paolo
>
>

Maybe just initialize iov unconditionally at the beginning and check
dot1q_buf instead of iov for the rest of the functions. (Need deal with
size < ETHER_ADDR_LEN * 2)