Re: [Qemu-devel] [PATCH] 9pfs: fix potential segfault during walk

[Cédric Le Goater]

On 09/16/2016 09:19 AM, Greg Kurz wrote:
> On Fri, 16 Sep 2016 01:05:11 +0200
> Greg Kurz <address@hidden> wrote:
> 
>> If the call to fid_to_qid() returns an error, we will call v9fs_path_free()
>> on uninitialized paths.
>>
> 
> I'll add this to the changelog:
> 
> It is a regression introduced by the following commit:
> 
> 56f101ecce0e 9pfs: handle walk of ".." in the root directory
> 
>> Let's fix this by initializing dpath and path before calling fid_to_qid().
>>
>> Signed-off-by: Greg Kurz <address@hidden>
>> ---
>>
>> Thanks Paolo (and Coverity) for spotting this.
>>
>> Cc'ing stable as this is a regression introduced in 2.7. It is also present
>> in Michael's stable-2.6-staging branch.
>>
>>  hw/9pfs/9p.c |    5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>> index dfe293d11d1c..91a497079acb 100644
>> --- a/hw/9pfs/9p.c
>> +++ b/hw/9pfs/9p.c
>> @@ -1320,13 +1320,14 @@ static void v9fs_walk(void *opaque)
>>          goto out_nofid;
>>      }
>>  
>> +    v9fs_path_init(&dpath);
>> +    v9fs_path_init(&path);
>> +
>>      err = fid_to_qid(pdu, fidp, &qid);
>>      if (err < 0) {
>>          goto out;
>>      }
>>  
>> -    v9fs_path_init(&dpath);
>> -    v9fs_path_init(&path);
>>      /*
>>       * Both dpath and path initially poin to fidp.
>>       * Needed to handle request with nwnames == 0
>>
> 
> 


There is still a possible segv I think, in out_nofid :

    if (nwnames && nwnames <= P9_MAXWELEM) {
        for (name_idx = 0; name_idx < nwnames; name_idx++) {
            v9fs_string_free(&wnames[name_idx]);
        
&wnames[name_idx] could NULL if pdu_unmarshal() fails.



C.