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

Re: [PATCH 1/2] x86/time: use fake read_tsc()


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Tue, 1 Mar 2022 13:07:27 +0000
  • Accept-language: en-GB, en-US
  • 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=YJ+fCSm8D5VU+uZHrw75Nj2/sezhnaA0OYiRp42QpAc=; b=UQlZWqeR4O+iiK2t4iIcbgJQdWJPHbd5hfu2wtKWZ2q854+TTQjy2crAbU82ll+IHhdzUidMtbS/WNu9nbbfvyD/yXntZRPozsOSzjjZC7+fLWRw4d+iHK/JJfsAsOL0k0fPDu5k4yohU2MDhGUe3s7E96AXxYhEpE7Is5v2nmduTYuXe3KjLp8sBp72QeWiGQj6YlQPLhUa1M3YVvfcxjwsegIFYNn/PBz6WtsvkaH5Zzy7PdneSmcNvvGUWwwy3vxH+DJybSwVEaL8332JEpLpHh+kyyeeNNp2fVLIG59f7jA9hC8mFsJ9YpPM1TYQS6xAnQQUBt0r6YJbob4MMw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dAB1IFVaTnaiACZUcTKt+5FQY4OkRFZEPSjfa+zGWJnIKNr8dcZLT/nNtt8hO2ht29YHIn9Mff6jNisLP5In9jZz7Qwg0nf/EYFMBkgw867/WHCGRvPoBD/iJ3Rs9VhBNPGw4VZ5mhm3iaxAjlLzyEKU7VS8iMGvoT21GM37R81VLfeUzPx5ddSW+4tG9Nxn2WumnCeQu22yJzcRB12aeOQJE4tPUGsvGoiUW+23gJAHXVcbS1JdicedhlXMpjgIUk2nG0F1NAT9VTj8d+kFcgff0X7+UJK9AqKBdY2yKfJS9zTelhvf2m6X3h/I0XIf2eOCGiN1l15RNW8EKxNkeQ==
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • Delivery-date: Tue, 01 Mar 2022 13:07:39 +0000
  • Ironport-data: A9a23:WFJ8Fa7UNWxHrQJ6/JVqugxRtCXHchMFZxGqfqrLsTDasY5as4F+v mRMXDyGaf3YMDGge993YISz/RtUv5/cy4RiSAdrrXsyHi5G8cbLO4+Ufxz6V8+wwmwvb67FA +E2MISowBUcFyeEzvuVGuG96yE6j8lkf5KkYAL+EnkZqTRMFWFw0XqPp8Zj2tQy2YPhWlvX0 T/Pi5a31GGNimYc3l08s8pvmDs31BglkGpF1rCWTakjUG72zxH5PrpGTU2CByKQrr1vNvy7X 47+IISRpQs1yfuP5uSNyd4XemVSKlLb0JPnZnB+A8BOiTAazsA+PzpS2FPxpi67hh3Q9+2dx umhurSNViczJ5bOm9gDVkFXExlYHpRJ3pr+dC3XXcy7lyUqclPpyvRqSko3IZcZ6qB8BmQmG f4wcW5XKErZ3qTvnez9GrIEascLdaEHOKs2vH16wC6fJvEhWZ3ZGI3B5MNC3Sd2jcdLdRrbT 5RFN2cxPUieC/FJEgkyS5IPw8T1ulLiNARdj2yYhbp03FGGmWSd15CyaYGIK7RmX/59gUKwt m/AuWPjDXkyJNGZjDaI7H+oruvOhj/gHpIfEqWi8fxni0HVwXYcYDUUX1ampfiyimalRslSb UcT/0ITQbMarRLxCIOnBlvh/SDC7kV0t8ds//MS1R6t0LPv4QWlGG1cQT55Tv8fu+kTfGl/v rOWpO/BCTtqubyTbHuS8LaIsD+/URQowX8+iTwsFlVcvYS6yG0npleWF4s4Tvbp5jHgMWyom 1i3QD4Ca6L/ZCLh/4Gy5hj5jj2lvfAlpSZlt1yMDgpJAu6UDbNJhrBEC3CGtZ6sz67DFzFtW UTofeDEtIji6rnXyUSwrB0lRu3B2hp8GGS0baRTN5cg7S+x3HWoYJpd5jpzTG8wbJpaIWCyP hOK4FoAjHO2AJdMRfUsC25WI553pZUM6Py/DqyEBjawSsIZmPC7ENFGOhfLgjGFfLkEmqAjI 5aLGftA/l5BYZmLOAGeHr9HuZdyn3hW7TqKGfjTkkT2uZLDNSX9YepUbzOzghURsfrsTPP9q I0EaaNnCnx3DYXDX8Ug2dVLfABScCNiXsieRg4+XrfrHzeK0VoJUpf56bggZ5Zkj+JSkOLJ9 Wu6QUhW1Bz0gnivFOlAQioLhG/HNXqnkU8GAA==
  • Ironport-hdrordr: A9a23:IRqloqBq022owpLlHegCsceALOsnbusQ8zAXPh9KJiC9I/b1qy nxppkmPEfP+UsssHFJo6HkBEEZKUmsuqKdkrNhQYtKOzOW9ldATbsSobcKpgePJ8SQzJ8l6U 4NSdkcNDS0NykBsS+Y2nj4Lz9D+qj+zEnAv463pB0NLT2CKZsQlDuRYjzrSXGeLzM2YabRYa DsgPav0ADQHkj/AP7LZEUtbqzmnZnmhZjmaRkJC1oM8w+Vlw6l77b8Dlyxwgoeeykn+8ZjzU H11yjCoomzufCyzRHRk0XJ6Y5NpdfnwtxfQOSRl8kuLCn2gArAXvUjZ1TChkF2nAic0idvrD D+mWZmAy210QKWQoiBm2qp5+An6kd215at8y7BvZKpm72HeNtzMbs+uWseSGqC16NohqAN7E oAtVjpxqZ/HFfOmj/w6MPPUAwvnk2ooWA6mepWlHBHV5ACAYUh5rD30XklWavoJhiKoLzP0d MeeP309bJTaxeXfnrZtm5gzJilWWkyBA6PRgwHttaO2zZbkXhlxw9ArfZv00so5dY4Ud1J9u 7EOqNnmPVHSdIXd7t0AKMETdGsAmLATBrQOCaZIEjhFqsAJ3XRwqSHqokd9aWvYtgF3ZEykJ POXBdRsnMzYVvnDYmU0JhC4nn2MROAtPTWu7ZjDrRCy8/BreDQQF6+oXgV4r6dn8k=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYLVxVKor9YtEUbkuWag9T3yUcNqyqf+CA
  • Thread-topic: [PATCH 1/2] x86/time: use fake read_tsc()

On 01/03/2022 11:05, Jan Beulich wrote:
> Go a step further than bed9ae54df44 ("x86/time: switch platform timer
> hooks to altcall") did and eliminate the "real" read_tsc() altogether:
> It's not used except in pointer comparisons, and hence it looks overall
> more safe to simply poison plt_tsc's read_counter hook.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> I wasn't really sure whether it would be better to use simply void * for
> the type of the expression, resulting in an undesirable data -> function
> pointer conversion, but making it impossible to mistakenly try and call
> the (fake) function directly.
>
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -585,10 +585,12 @@ static s64 __init cf_check init_tsc(stru
>      return ret;
>  }
>  
> -static uint64_t __init cf_check read_tsc(void)
> -{
> -    return rdtsc_ordered();
> -}
> +/*
> + * plt_tsc's read_counter hook is (and should not be) invoked via the struct

Either "is not (and should not be)" or "is (and should) not be".

> + * field. To avoid carrying an unused, indirectly reachable function, poison
> + * the field with an easily identifiable non-canonical pointer.
> + */
> +#define read_tsc ((uint64_t(*)(void))0x75C75C75C75C75C0ul)

How about using ZERO_BLOCK_PTR?  The hex constant 0xBAD0... is more
easily recognisable, and any poisoned pointer will do.

That said... what's wrong a plain NULL?  I can't see any need for a
magic constant here.


Overall, I think this patch should be merged with the subsequent one,
because in isolation it is slightly dubious.  read_tsc() is one of the
few functions which is of no interest to an attacker, architecturally
(because it's just rdtsc) or speculatively (because it is dispatch
serialising).

This change is only (AFAICT) to allow the use of cf_clobber later.

~Andrew

 


Rackspace

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