[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH][4.15] gnttab: work around "may be used uninitialized" warning


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 12 Mar 2021 13:25:18 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Uwt255OP6dwuelu+TJrjEQQibtbltatYNSr7c26diKE=; b=VlkFShxZBbR4lQ69trMtR8gvadEzY7i17jWuUGxdb1fAVWfVsBMZPL+yDm5IYxuEF+SnaMn1994LK7JVB6o3d2+2lVaRwFlQBgZAbBBTWwLdovju0ItEE55N/esTcGSk1AaSZ5GgTD7LxPizA0+BWtuxUMD3r3zoaWK857Zvagkca4pLZhmvziyORNEzjZaPz2eGyhuVwI1BMyZ4nGxhgeq/RKt6BveM3n04LGuq7jRoIXAUsYBNJ6m2vhDDbpEGYjjR1HX0gc4ThbLM9gYoWvBQZjgH3eyvsjxFAeTlbEJWf0r6mYqNnMuVDBVxYAlFVgCfZlic80cK2Em8k/S9+w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XVPnCPHSBZxlwk8WToU/xTtEarTKilbmgijvTUisz6l1+/WfwPYRdUaHH9RAuhrYIfg7/6upNzQKwYF3IozGD4g5nyBdeqB2oJCp0I5saNqaISBeEyk8bPLWu6JUM3LM29VCN4g+/9CAhGv6oAFeCAI9xHpCAMhhyIUDj/Sybp1xnpcmY+runSCYz0xkcfFOAvl1gtfQ6wfm02XXitOO6nMpy2mM3pFQXhJFlqzR8EAgoC/NHqFJtTys6sfRCQKoLKscJsBX7CfOLw/0wtRTMm+C1neMw1vw/Wo1lVWtEYZrI6zZIvXRS8XTgQTP7jGsPoOcmoe0oNBw2CyO+mXB7Q==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 12 Mar 2021 13:25:43 +0000
  • Ironport-hdrordr: A9a23:1VX9m6Pohb+tscBcT2zw55DYdL4zR+YMi2QD/3taDTRIb82VkN 2vlvwH1RnyzA0cQm0khMroAsa9aFvm39pQ7ZMKNbmvGDPntmyhMZ144eLZrwHIMxbVstRQ3a IIScVDIfXtEFl3itv76gGkE9AmhOKK6rysmP229RZQZCtBApsQiztRIACdD0FwWU1iDZ02CJ KT6qN81kWdUF4Qadm2AWRAYvPKoMfFmImjTRkNARMm7wfmt0LV1JfRFR+E0hACFw5e2LtKyx m5ryXVxIWG98u6xBjVynPJ4/1t9ufJ59NfCKW3+7AoAxr2jALAXvUGZ5Sju3QPrPir+BIWlr D30m0dFuBSz1+UQW2vuxvq3GDboUUTwlvv00WRj3emgeGRfkNCN+N7iYhUcgTU5iMb1bkWus I7vBPti7NtARzNhyj77dTTPisa8XacmnY+jfUVy0VWTIp2Us4gkaUk4EhXHJ0cdRiKirwPLe 8GNrC42N9ra1+AK1jWsm5zqebcJUgbL1OtR0gPvdGtyD5GnHx15Ftw/r1vol4wsL06UJVK/O LCL+BBk6xPVNYfaeZHCP4GWtbfMB2DfTv8dEapZXj3HqAOPHzA77bx/bUO/emvPLgF1oE7lp jtWE5R3FRCNX7GOImr5tlm4xrNSGKyUXDG0cdF/aV0vbX6Wf7CLTCDYEpGqbrin9wvRungH9 qjMpNfBPHuaUH0H5xS4gH4U55ObVEDTcwuvMohUV7mmLOKFqTa8sjgNNrDLrvkFjgpHknlBG EYYTT1LMJcqm+xXHvVhwXQRmPNdkTz8YkYKtmew8EjjKw2cqFcuAkcjlq0ouuRLydZj6AwdE xiZJPr+5nL4VWezCLt1SFEKxBdBkFa7PHLSHVRvzIHNEvybPIms9WbcmZC4WufKnZEPoTrOT 8ag24y1bO8LpSWyyxnIcmgKHimg3wao2/PaJsAhKuZ54PAdokjBpgrHIx9fD+7ViBdqEJPki NueQUETkjQGnfFkqO+lqEZA+nZap1bmwekIcldrFrFrkWCrcQTRn8WNgTeE/K/sEILfX55l1 dx+6gQjP6rgjC0M1Yyh+w+LRlxcmiNOalHCw6EfY1QvbjudGhLPCG3rA3fryt2Vnvh9k0UiG CkCSGPY/nEDmBQvW1i3r/w/El5cXiceExMeml32LcNZ1juizJW66umd6Cz22yeZh85zuYRPC rsTBESLgltrurHniK9qXKnLzEL158uNuvSAPAfaLnVwGqqM5DNv7oBBeVo8JFsM83OvucHXf mEQRKcKCr1BooSqlWoj0dgHBMxhGgvkPvu1hGg0XOx22QnB+HOZHthXLMWLrinniHZbsfN9K 88q907veG9aDqsLvGHzLzadD5FJFf4p3WsQ+QhtJBTuuYTudJIbu7meAqN8EsC+hM0aPrQvg c5Zo9Q5bjaII9hf8AIYUtijxEUveXKCHFuixD8B+81QEokgHDaNe6Y+ragk8taPmSx4C/LfW SF+yJT//35TzKO+L4TBaU3O3lXYiEHmQJf1dLHU43bEwOxce5fuHK8L3+mabdYIZL1VIk4n1 Jf49uSmfWQeDe98AfMvSFjKqYL12q8W8u9DEatHuFPmubKdWiks++P4MSpii3wRib+Q0MEhZ ddfUhVV/99sFAZ/cUK+xn3bLf2rEIjm0Zf5j8itmeF4PnZ3E7rWWdcMQPYhZ1KWyJ0KXbgt7 WczdSl
  • Ironport-sdr: hcYBs6402z7ru3IdTAu71+W97MMJBDsTliZ0ZRkr11agwfEb3mRhbuQHbxuZ7KgV93P447bcjV Gcr4eyHLKIr8St7krbmaGcgM6TRvwOu7Cx6ZjS29dNQUqPjl9AlGEX2rNSpDi7uVzsiGGEqlON 4RyKP4DP0qia+MqPIIoFzVVAGQcu+vjQ9z5+9le+c6KA5yCtiZbRuUjWrF2tKrdMV0TkGlctC+ iI0aCCD8kbx4tk0H89jYpxHx4VtTHBHzXc2Jkj/R1yH3hcqJpYPsQV7DMaP059hyUZtc5cbZ7J jwU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 12/03/2021 13:08, Jan Beulich wrote:
> On 12.03.2021 12:32, Andrew Cooper wrote:
>> On 10/03/2021 10:13, Jan Beulich wrote:
>>> Sadly I was wrong to suggest dropping vaddrs' initializer during review
>>> of v2 of the patch introducing this code. gcc 4.3 can't cope.
>>>
>>> Fixes: 52531c734ea1 ("xen/gnttab: Rework resource acquisition")
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -4026,7 +4026,7 @@ int gnttab_acquire_resource(
>>>      struct grant_table *gt = d->grant_table;
>>>      unsigned int i, final_frame;
>>>      mfn_t tmp;
>>> -    void **vaddrs;
>>> +    void **vaddrs = NULL;
>>>      int rc = -EINVAL;
>>>  
>>>      if ( !nr_frames )
>> in v1, there was a companion check.
>>
>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>> index f937c1d350..2bb07f129f 100644
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -4059,6 +4059,16 @@ int gnttab_acquire_resource(
>>      if ( rc )
>>          goto out;
>>  
>> +    /*
>> +     * Some older toolchains can't spot that vaddrs is non-NULL on
>> non-error
>> +     * paths.  Leave some runtime safety.
>> +     */
>> +    if ( !vaddrs )
>> +    {
>> +        ASSERT_UNREACHABLE();
>> +        goto out;
>> +    }
>> +
>>      for ( i = 0; i < nr_frames; ++i )
>>          mfn_list[i] = virt_to_mfn(vaddrs[frame + i]);
> Oh, I didn't realize this. Will add, but did you really mean to
> have the function return success in this case (on a release
> build)? I'd be inclined to put it ahead of if "if ( rc )" and
> set rc (to e.g. -ENODATA) in this case.

Oh - quite right.  Returning 0 here will hit the assertion/failsafe
protecting against livelock.

I'd be tempted to chose -EINVAL because the only plausible way to get
here is a bad id, and that path should have errored out earlier.

And yeah, with the rc adjustment, fine to reposition.

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.