All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
@ 2015-12-21 23:10 Eric Shelton
  2015-12-22 12:24 ` Stefano Stabellini
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Shelton @ 2015-12-21 23:10 UTC (permalink / raw)
  To: xen-devel, Stefano Stabellini, Samuel Thibault, security


[-- Attachment #1.1: Type: text/plain, Size: 8792 bytes --]

Seeing as "All OSes providing PV backends are susceptible," doesn't this
include MiniOS for QEMU stubdom as well?  Are there patches available for
mini-os/blkfront.c, mini-os/netfront.c, and mini-os/pcifront.c?  I didn't
see anything for this.

Best,
Eric

On Thu, Dec 17, 2015 at 1:36 PM, Xen.org security team <security@xen.org>
wrote:

>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>             Xen Security Advisory CVE-2015-8550 / XSA-155
>                               version 6
>
>     paravirtualized drivers incautious about shared memory contents
>
> UPDATES IN VERSION 6
> ====================
>
> Correct CREDITS section.
>
> ISSUE DESCRIPTION
> =================
>
> The compiler can emit optimizations in the PV backend drivers which
> can lead to double fetch vulnerabilities. Specifically the shared
> memory between the frontend and backend can be fetched twice (during
> which time the frontend can alter the contents) possibly leading to
> arbitrary code execution in backend.
>
> IMPACT
> ======
>
> Malicious guest administrators can cause denial of service.  If driver
> domains are not in use, the impact can be a host crash, or privilege
> escalation.
>
> VULNERABLE SYSTEMS
> ==================
>
> Systems running PV or HVM guests are vulnerable.
>
> ARM and x86 systems are vulnerable.
>
> All OSes providing PV backends are susceptible, this includes
> Linux and NetBSD. By default the Linux distributions compile kernels
> with optimizations.
>
> MITIGATION
> ==========
>
> There is no mitigation.
>
> CREDITS
> =======
>
> This issue was discovered by Felix Wilhelm (ERNW Research, KIT /
> Operating Systems Group).
>
> RESOLUTION
> ==========
>
> Applying the appropriate attached patches should fix the problem for
> PV backends.  Note only that PV backends are fixed; PV frontend
> patches will be developed and released (publicly) after the embargo
> date.
>
> Please note that there is a bug in some versions of gcc,
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 which can cause the
> construct used in RING_COPY_REQUEST() to be ineffective in some
> circumstances. We have determined that this is only the case when the
> structure being copied consists purely of bitfields. The Xen PV
> protocols updated here do not use bitfields in this way and therefore
> these patches are not subject to that bug. However authors of third
> party PV protocols should take this into consideration.
>
> Linux v4.4:
> xsa155-linux-xsa155-0001-xen-Add-RING_COPY_REQUEST.patch
>
> xsa155-linux-xsa155-0002-xen-netback-don-t-use-last-request-to-determine-mini.patch
> xsa155-linux-xsa155-0003-xen-netback-use-RING_COPY_REQUEST-throughout.patch
>
> xsa155-linux-xsa155-0004-xen-blkback-only-read-request-operation-from-shared-.patch
>
> xsa155-linux-xsa155-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
> xsa155-linux-xsa155-0006-xen-scsiback-safely-copy-requests.patch
>
> xsa155-linux-xsa155-0007-xen-pciback-Save-xen_pci_op-commands-before-processi.patch
> Linux v4.[0,1,2,3]
> All the above patches except #5 will apply, please use:
>
> xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
> Linux v3.19:
> All the above patches except #5 and #6 will apply, please use:
>
> xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
> xsa155-linux319-0006-xen-scsiback-safely-copy-requests.patch
>
> qemu-xen:
> xsa155-qemu-qdisk-double-access.patch
> xsa155-qemu-xenfb.patch
>
> qemu-traditional:
> xsa155-qemut-qdisk-double-access.patch
> xsa155-qemut-xenfb.patch
>
> NetBSD 7.0:
> xsa155-netbsd-xsa155-0001-netbsd-xen-Add-RING_COPY_REQUEST.patch
>
> xsa155-netbsd-xsa155-0002-netbsd-netback-Use-RING_COPY_REQUEST-instead-of-RING.patch
>
> xsa155-netbsd-xsa155-0003-netbsd-ring-Add-barrier-to-provide-an-compiler-barri.patch
>
> xsa155-netbsd-xsa155-0004-netbsd-block-only-read-request-operation-from-shared.patch
>
> xsa155-netbsd-xsa155-0005-netbsd-pciback-Operate-on-local-version-of-xen_pci_o.patch
>
> xen:
> xsa155-xen-0001-xen-Add-RING_COPY_REQUEST.patch
> xsa155-xen-0002-blktap2-Use-RING_COPY_REQUEST.patch
> xsa155-xen-0003-libvchan-Read-prod-cons-only-once.patch
>
> xen 4.4:
> All patches except #3 will apply, please use:
> xsa155-xen44-0003-libvchan-Read-prod-cons-only-once.patch
>
> $ sha256sum xsa155*
> d9fbc104ab2ae797971e351ee0e04e7b7e9c7c33385309bb406c7941dc9a33b4
> xsa155-linux319-xsa155-0006-xen-scsiback-safely-copy-requests.patch
> 590656d83ad7b6052b54659eccb3469658b3942c0dc1366423a66f2f5ac643e1
> xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
> 2bd18632178e09394c5cd06aded2c14bcc6b6e360ad6e81827d24860fe3e8ca4
> xsa155-linux-xsa155-0001-xen-Add-RING_COPY_REQUEST.patch
> cecdeccb8e2551252c81fc5f164a8298005df714a574a7ba18b84e8ed5f2bb70
> xsa155-linux-xsa155-0002-xen-netback-don-t-use-last-request-to-determine-mini.patch
> 3916b847243047f0e1053233ade742c14a7f29243584e60bf5db4842a8068855
> xsa155-linux-xsa155-0003-xen-netback-use-RING_COPY_REQUEST-throughout.patch
> 746c8eb0aeb200d76156c88dfbbd49db79f567b88b07eda70f7c7d095721f05a
> xsa155-linux-xsa155-0004-xen-blkback-only-read-request-operation-from-shared-.patch
> 18517a184a02f7441065b8d3423086320ec4c2345c00d551231f7976381767f5
> xsa155-linux-xsa155-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
> 2e6d556d25b1cc16e71afde665ae3908f4fa8eab7e0d96283fc78400301baf92
> xsa155-linux-xsa155-0006-xen-scsiback-safely-copy-requests.patch
> 5e130d8b61906015c6a94f8edd3cce97b172f96a265d97ecf370e7b45125b73d
> xsa155-linux-xsa155-0007-xen-pciback-Save-xen_pci_op-commands-before-processi.patch
> 08c2d0f95dcc215165afbce623b6972b81dd45b091b5f40017579b00c8612e03
> xsa155-netbsd-xsa155-0001-netbsd-xen-Add-RING_COPY_REQUEST.patch
> 0a66010f736092f91f70bb0fd220685e4395efef1db6d23a3d1eace31d144f51
> xsa155-netbsd-xsa155-0002-netbsd-netback-Use-RING_COPY_REQUEST-instead-of-RING.patch
> 5e913a8427cab6b4d384d1246e05116afc301eb117edd838101eb53a82c2f2ff
> xsa155-netbsd-xsa155-0003-netbsd-ring-Add-barrier-to-provide-an-compiler-barri.patch
> 3b8f14eafaed3a7bc66245753a37af4249acf8129fbedb70653192252dc47dc9
> xsa155-netbsd-xsa155-0004-netbsd-block-only-read-request-operation-from-shared.patch
> 81ae5fa998243a78dad749fc561be647dc1dc1be799e8f18484fdf0989469705
> xsa155-netbsd-xsa155-0005-netbsd-pciback-Operate-on-local-version-of-xen_pci_o.patch
> 044ff74fa048df820d528f64f2791ec9cb3940bd313c1179020bd49a6cde2ca3
> xsa155-qemu-qdisk-double-access.patch
> 1150504589eb7bfa108c80ce63395e57d0e627b12d9201219d968fdd026919a6
> xsa155-qemut-qdisk-double-access.patch
> 63186246ab6913b54bfef5f09f33e815935ac40ff821c27a3efda62339bbbd5f
> xsa155-qemut-xenfb.patch
> e53b4ac298648cde79344192d5a58ca8d8724344f5105bec7c09eef095c668f6
> xsa155-qemu-xenfb.patch
> e52467fcec73bcc86d3e96d06f8ca8085ae56a83d2c42a30c16bc3dc630d8f8a
> xsa155-xen-0001-xen-Add-RING_COPY_REQUEST.patch
> eae34c8ccc096ad93a74190506b3d55020a88afb0cc504a3a514590e9fd746fd
> xsa155-xen-0002-blktap2-Use-RING_COPY_REQUEST.patch
> 42780265014085a4221ad32b026214693d751789eb5219e2e83862c0006c66f4
> xsa155-xen-0003-libvchan-Read-prod-cons-only-once.patch
> dfcaddb8a908a4fc1b048a43187e885117e67dc566f5c841037ee366dcd437d1
> xsa155-xen44-0003-libvchan-Read-prod-cons-only-once.patch
> $
>
> DEPLOYMENT DURING EMBARGO
> =========================
>
> Deployment of the patches and/or mitigations described above (or
> others which are substantially similar) is permitted during the
> embargo, even on public-facing systems with untrusted guest users and
> administrators.
>
> But: Distribution of updated software is prohibited (except to other
> members of the predisclosure list).
>
> Predisclosure list members who wish to deploy significantly different
> patches and/or mitigations, please contact the Xen Project Security
> Team.
>
> (Note: this during-embargo deployment notice is retained in
> post-embargo publicly released Xen Project advisories, even though it
> is then no longer applicable.  This is to enable the community to have
> oversight of the Xen Project Security Team's decisionmaking.)
>
> For more information about permissible uses of embargoed information,
> consult the Xen Project community's agreed Security Policy:
>   http://www.xenproject.org/security-policy.html
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.12 (GNU/Linux)
>
> iQEcBAEBAgAGBQJWcrpdAAoJEIP+FMlX6CvZ9soIALqQ/GHP6bZn2LqJTD9DIzsm
> zVB4yCPiVfDqHSOq9QNCzBzqpvOX+RhKTzRH1jsZczr8CSnkePxaCrmZgH8SAygB
> hFcF9xJGlJDjs647sgpQmYs++3mgD/57uml7IW/8NX46tXUelVByW7muNgUN2xlm
> kjeD8auJEs+jK1iwpt/hOmYe4moRx3+3ujfgqMCNAWtqZz9D9wM5tao+p6yKYlhM
> u8hSi1V3b7sAbf92mwzpzfpbwdgg25xeHtZ/oJxp/ZY0FhqDEsTxV+h8HjD/Eink
> GwqPS19O77tMmz9fUUTyJDSsU7ayFRI0HyYmXju4eJktJkhXagjAdCSyGky9z5g=
> =FlX2
> -----END PGP SIGNATURE-----
>

[-- Attachment #1.2: Type: text/html, Size: 9922 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
  2015-12-21 23:10 Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory Eric Shelton
@ 2015-12-22 12:24 ` Stefano Stabellini
  2015-12-22 13:19   ` Samuel Thibault
  2015-12-22 15:06   ` Eric Shelton
  0 siblings, 2 replies; 9+ messages in thread
From: Stefano Stabellini @ 2015-12-22 12:24 UTC (permalink / raw)
  To: Eric Shelton; +Cc: Samuel Thibault, Stefano Stabellini, security, xen-devel

[-- Attachment #1: Type: text/plain, Size: 10439 bytes --]

MiniOS for QEMU stubdom has frontends, such as mini-os/blkfront.c and
mini-os/netfront.c, not backends.

Cheers,

Stefano


On Mon, 21 Dec 2015, Eric Shelton wrote:
> Seeing as "All OSes providing PV backends are susceptible," doesn't this include MiniOS for QEMU stubdom as well? 
> Are there patches available for mini-os/blkfront.c, mini-os/netfront.c, and mini-os/pcifront.c?  I didn't see
> anything for this.
> Best,
> Eric
>
> On Thu, Dec 17, 2015 at 1:36 PM, Xen.org security team <security@xen.org> wrote:
>
>       ----- Topal: Output generated on Tue Dec 22 12:23:44 GMT 2015 ----- Topal: GPG output starts ----- gpg:
>       no valid OpenPGP data found. gpg: processing message failed: eof ----- Topal: GPG output ends -----
>       ----- Topal: Original message starts ----- -----BEGIN PGP SIGNED MESSAGE-----
>       Hash: SHA1
>
>                   Xen Security Advisory CVE-2015-8550 / XSA-155
>                                     version 6
>
>           paravirtualized drivers incautious about shared memory contents
>
>       UPDATES IN VERSION 6
>       ====================
>
>       Correct CREDITS section.
>
>       ISSUE DESCRIPTION
>       =================
>
>       The compiler can emit optimizations in the PV backend drivers which
>       can lead to double fetch vulnerabilities. Specifically the shared
>       memory between the frontend and backend can be fetched twice (during
>       which time the frontend can alter the contents) possibly leading to
>       arbitrary code execution in backend.
>
>       IMPACT
>       ======
>
>       Malicious guest administrators can cause denial of service.  If driver
>       domains are not in use, the impact can be a host crash, or privilege escalation.
>
>       VULNERABLE SYSTEMS
>       ==================
>
>       Systems running PV or HVM guests are vulnerable.
>
>       ARM and x86 systems are vulnerable.
>
>       All OSes providing PV backends are susceptible, this includes
>       Linux and NetBSD. By default the Linux distributions compile kernels
>       with optimizations.
>
>       MITIGATION
>       ==========
>
>       There is no mitigation.
>
>       CREDITS
>       =======
>
>       This issue was discovered by Felix Wilhelm (ERNW Research, KIT /
>       Operating Systems Group).
>
>       RESOLUTION
>       ==========
>
>       Applying the appropriate attached patches should fix the problem for
>       PV backends.  Note only that PV backends are fixed; PV frontend
>       patches will be developed and released (publicly) after the embargo
>       date.
>
>       Please note that there is a bug in some versions of gcc,
>       https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 which can cause the
>       construct used in RING_COPY_REQUEST() to be ineffective in some
>       circumstances. We have determined that this is only the case when the
>       structure being copied consists purely of bitfields. The Xen PV
>       protocols updated here do not use bitfields in this way and therefore
>       these patches are not subject to that bug. However authors of third
>       party PV protocols should take this into consideration.
>
>       Linux v4.4:
>       xsa155-linux-xsa155-0001-xen-Add-RING_COPY_REQUEST.patch
>       xsa155-linux-xsa155-0002-xen-netback-don-t-use-last-request-to-determine-mini.patch
>       xsa155-linux-xsa155-0003-xen-netback-use-RING_COPY_REQUEST-throughout.patch
>       xsa155-linux-xsa155-0004-xen-blkback-only-read-request-operation-from-shared-.patch
>       xsa155-linux-xsa155-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>       xsa155-linux-xsa155-0006-xen-scsiback-safely-copy-requests.patch
>       xsa155-linux-xsa155-0007-xen-pciback-Save-xen_pci_op-commands-before-processi.patch
>       Linux v4.[0,1,2,3]
>       All the above patches except #5 will apply, please use:
>       xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>       Linux v3.19:
>       All the above patches except #5 and #6 will apply, please use:
>       xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>       xsa155-linux319-0006-xen-scsiback-safely-copy-requests.patch
>
>       qemu-xen:
>       xsa155-qemu-qdisk-double-access.patch
>       xsa155-qemu-xenfb.patch
>
>       qemu-traditional:
>       xsa155-qemut-qdisk-double-access.patch
>       xsa155-qemut-xenfb.patch
>
>       NetBSD 7.0:
>       xsa155-netbsd-xsa155-0001-netbsd-xen-Add-RING_COPY_REQUEST.patch
>       xsa155-netbsd-xsa155-0002-netbsd-netback-Use-RING_COPY_REQUEST-instead-of-RING.patch
>       xsa155-netbsd-xsa155-0003-netbsd-ring-Add-barrier-to-provide-an-compiler-barri.patch
>       xsa155-netbsd-xsa155-0004-netbsd-block-only-read-request-operation-from-shared.patch
>       xsa155-netbsd-xsa155-0005-netbsd-pciback-Operate-on-local-version-of-xen_pci_o.patch
>
>       xen:
>       xsa155-xen-0001-xen-Add-RING_COPY_REQUEST.patch
>       xsa155-xen-0002-blktap2-Use-RING_COPY_REQUEST.patch
>       xsa155-xen-0003-libvchan-Read-prod-cons-only-once.patch
>
>       xen 4.4:
>       All patches except #3 will apply, please use:
>       xsa155-xen44-0003-libvchan-Read-prod-cons-only-once.patch
>
>       $ sha256sum xsa155*
>       d9fbc104ab2ae797971e351ee0e04e7b7e9c7c33385309bb406c7941dc9a33b4 
>       xsa155-linux319-xsa155-0006-xen-scsiback-safely-copy-requests.patch
>       590656d83ad7b6052b54659eccb3469658b3942c0dc1366423a66f2f5ac643e1 
>       xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>       2bd18632178e09394c5cd06aded2c14bcc6b6e360ad6e81827d24860fe3e8ca4 
>       xsa155-linux-xsa155-0001-xen-Add-RING_COPY_REQUEST.patch
>       cecdeccb8e2551252c81fc5f164a8298005df714a574a7ba18b84e8ed5f2bb70 
>       xsa155-linux-xsa155-0002-xen-netback-don-t-use-last-request-to-determine-mini.patch
>       3916b847243047f0e1053233ade742c14a7f29243584e60bf5db4842a8068855 
>       xsa155-linux-xsa155-0003-xen-netback-use-RING_COPY_REQUEST-throughout.patch
>       746c8eb0aeb200d76156c88dfbbd49db79f567b88b07eda70f7c7d095721f05a 
>       xsa155-linux-xsa155-0004-xen-blkback-only-read-request-operation-from-shared-.patch
>       18517a184a02f7441065b8d3423086320ec4c2345c00d551231f7976381767f5 
>       xsa155-linux-xsa155-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>       2e6d556d25b1cc16e71afde665ae3908f4fa8eab7e0d96283fc78400301baf92 
>       xsa155-linux-xsa155-0006-xen-scsiback-safely-copy-requests.patch
>       5e130d8b61906015c6a94f8edd3cce97b172f96a265d97ecf370e7b45125b73d 
>       xsa155-linux-xsa155-0007-xen-pciback-Save-xen_pci_op-commands-before-processi.patch
>       08c2d0f95dcc215165afbce623b6972b81dd45b091b5f40017579b00c8612e03 
>       xsa155-netbsd-xsa155-0001-netbsd-xen-Add-RING_COPY_REQUEST.patch
>       0a66010f736092f91f70bb0fd220685e4395efef1db6d23a3d1eace31d144f51 
>       xsa155-netbsd-xsa155-0002-netbsd-netback-Use-RING_COPY_REQUEST-instead-of-RING.patch
>       5e913a8427cab6b4d384d1246e05116afc301eb117edd838101eb53a82c2f2ff 
>       xsa155-netbsd-xsa155-0003-netbsd-ring-Add-barrier-to-provide-an-compiler-barri.patch
>       3b8f14eafaed3a7bc66245753a37af4249acf8129fbedb70653192252dc47dc9 
>       xsa155-netbsd-xsa155-0004-netbsd-block-only-read-request-operation-from-shared.patch
>       81ae5fa998243a78dad749fc561be647dc1dc1be799e8f18484fdf0989469705 
>       xsa155-netbsd-xsa155-0005-netbsd-pciback-Operate-on-local-version-of-xen_pci_o.patch
>       044ff74fa048df820d528f64f2791ec9cb3940bd313c1179020bd49a6cde2ca3  xsa155-qemu-qdisk-double-access.patch
>       1150504589eb7bfa108c80ce63395e57d0e627b12d9201219d968fdd026919a6 
>       xsa155-qemut-qdisk-double-access.patch
>       63186246ab6913b54bfef5f09f33e815935ac40ff821c27a3efda62339bbbd5f  xsa155-qemut-xenfb.patch
>       e53b4ac298648cde79344192d5a58ca8d8724344f5105bec7c09eef095c668f6  xsa155-qemu-xenfb.patch
>       e52467fcec73bcc86d3e96d06f8ca8085ae56a83d2c42a30c16bc3dc630d8f8a 
>       xsa155-xen-0001-xen-Add-RING_COPY_REQUEST.patch
>       eae34c8ccc096ad93a74190506b3d55020a88afb0cc504a3a514590e9fd746fd 
>       xsa155-xen-0002-blktap2-Use-RING_COPY_REQUEST.patch
>       42780265014085a4221ad32b026214693d751789eb5219e2e83862c0006c66f4 
>       xsa155-xen-0003-libvchan-Read-prod-cons-only-once.patch
>       dfcaddb8a908a4fc1b048a43187e885117e67dc566f5c841037ee366dcd437d1 
>       xsa155-xen44-0003-libvchan-Read-prod-cons-only-once.patch
>       $
>
>       DEPLOYMENT DURING EMBARGO
>       =========================
>
>       Deployment of the patches and/or mitigations described above (or
>       others which are substantially similar) is permitted during the
>       embargo, even on public-facing systems with untrusted guest users and
>       administrators.
>
>       But: Distribution of updated software is prohibited (except to other
>       members of the predisclosure list).
>
>       Predisclosure list members who wish to deploy significantly different
>       patches and/or mitigations, please contact the Xen Project Security
>       Team.
>
>       (Note: this during-embargo deployment notice is retained in
>       post-embargo publicly released Xen Project advisories, even though it
>       is then no longer applicable.  This is to enable the community to have
>       oversight of the Xen Project Security Team's decisionmaking.)
>
>       For more information about permissible uses of embargoed information,
>       consult the Xen Project community's agreed Security Policy:
>         http://www.xenproject.org/security-policy.html
>       -----BEGIN PGP SIGNATURE-----
>       Version: GnuPG v1.4.12 (GNU/Linux)
>
>       iQEcBAEBAgAGBQJWcrpdAAoJEIP+FMlX6CvZ9soIALqQ/GHP6bZn2LqJTD9DIzsm
>       zVB4yCPiVfDqHSOq9QNCzBzqpvOX+RhKTzRH1jsZczr8CSnkePxaCrmZgH8SAygB
>       hFcF9xJGlJDjs647sgpQmYs++3mgD/57uml7IW/8NX46tXUelVByW7muNgUN2xlm
>       kjeD8auJEs+jK1iwpt/hOmYe4moRx3+3ujfgqMCNAWtqZz9D9wM5tao+p6yKYlhM
>       u8hSi1V3b7sAbf92mwzpzfpbwdgg25xeHtZ/oJxp/ZY0FhqDEsTxV+h8HjD/Eink
>       GwqPS19O77tMmz9fUUTyJDSsU7ayFRI0HyYmXju4eJktJkhXagjAdCSyGky9z5g=
>       =FlX2
>       -----END PGP SIGNATURE-----
>
> ----- Topal: Original message ends -----
> 

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
  2015-12-22 12:24 ` Stefano Stabellini
@ 2015-12-22 13:19   ` Samuel Thibault
  2015-12-22 15:06   ` Eric Shelton
  1 sibling, 0 replies; 9+ messages in thread
From: Samuel Thibault @ 2015-12-22 13:19 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, security, Eric Shelton

Stefano Stabellini, on Tue 22 Dec 2015 12:24:35 +0000, wrote:
> MiniOS for QEMU stubdom has frontends, such as mini-os/blkfront.c and
> mini-os/netfront.c, not backends.

There is one backend, tpmback.  It however doesn't use a ring.

Samuel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
  2015-12-22 12:24 ` Stefano Stabellini
  2015-12-22 13:19   ` Samuel Thibault
@ 2015-12-22 15:06   ` Eric Shelton
  2016-01-04 13:06     ` Marek Marczykowski-Górecki
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Shelton @ 2015-12-22 15:06 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: minios-devel, Samuel Thibault, security, xen-devel

The XSA mentions that "PV frontend patches will be developed and
released (publicly) after the embargo date."  Has anything been done
towards this that should also be incorporated into MiniOS?  On a
system utilizing a "driver domain," where a backend is running on a
domain that is considered unprivileged and untrusted (such as the
example described in http://wiki.xenproject.org/wiki/Driver_Domain),
it seems XSA-155-style double fetch vulnerabilities in the frontends
are also a potential security concern, and should be eliminated.
However, perhaps that does not include pcifront, since pciback would
always be running in dom0.

Eric

On Tue, Dec 22, 2015 at 7:24 AM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> MiniOS for QEMU stubdom has frontends, such as mini-os/blkfront.c and
> mini-os/netfront.c, not backends.
>
> Cheers,
>
> Stefano
>
>
> On Mon, 21 Dec 2015, Eric Shelton wrote:
>> Seeing as "All OSes providing PV backends are susceptible," doesn't this include MiniOS for QEMU stubdom as well?
>> Are there patches available for mini-os/blkfront.c, mini-os/netfront.c, and mini-os/pcifront.c?  I didn't see
>> anything for this.
>> Best,
>> Eric
>>
>> On Thu, Dec 17, 2015 at 1:36 PM, Xen.org security team <security@xen.org> wrote:
>>
>>       ----- Topal: Output generated on Tue Dec 22 12:23:44 GMT 2015 ----- Topal: GPG output starts ----- gpg:
>>       no valid OpenPGP data found. gpg: processing message failed: eof ----- Topal: GPG output ends -----
>>       ----- Topal: Original message starts ----- -----BEGIN PGP SIGNED MESSAGE-----
>>       Hash: SHA1
>>
>>                   Xen Security Advisory CVE-2015-8550 / XSA-155
>>                                     version 6
>>
>>           paravirtualized drivers incautious about shared memory contents
>>
>>       UPDATES IN VERSION 6
>>       ====================
>>
>>       Correct CREDITS section.
>>
>>       ISSUE DESCRIPTION
>>       =================
>>
>>       The compiler can emit optimizations in the PV backend drivers which
>>       can lead to double fetch vulnerabilities. Specifically the shared
>>       memory between the frontend and backend can be fetched twice (during
>>       which time the frontend can alter the contents) possibly leading to
>>       arbitrary code execution in backend.
>>
>>       IMPACT
>>       ======
>>
>>       Malicious guest administrators can cause denial of service.  If driver
>>       domains are not in use, the impact can be a host crash, or privilege escalation.
>>
>>       VULNERABLE SYSTEMS
>>       ==================
>>
>>       Systems running PV or HVM guests are vulnerable.
>>
>>       ARM and x86 systems are vulnerable.
>>
>>       All OSes providing PV backends are susceptible, this includes
>>       Linux and NetBSD. By default the Linux distributions compile kernels
>>       with optimizations.
>>
>>       MITIGATION
>>       ==========
>>
>>       There is no mitigation.
>>
>>       CREDITS
>>       =======
>>
>>       This issue was discovered by Felix Wilhelm (ERNW Research, KIT /
>>       Operating Systems Group).
>>
>>       RESOLUTION
>>       ==========
>>
>>       Applying the appropriate attached patches should fix the problem for
>>       PV backends.  Note only that PV backends are fixed; PV frontend
>>       patches will be developed and released (publicly) after the embargo
>>       date.
>>
>>       Please note that there is a bug in some versions of gcc,
>>       https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 which can cause the
>>       construct used in RING_COPY_REQUEST() to be ineffective in some
>>       circumstances. We have determined that this is only the case when the
>>       structure being copied consists purely of bitfields. The Xen PV
>>       protocols updated here do not use bitfields in this way and therefore
>>       these patches are not subject to that bug. However authors of third
>>       party PV protocols should take this into consideration.
>>
>>       Linux v4.4:
>>       xsa155-linux-xsa155-0001-xen-Add-RING_COPY_REQUEST.patch
>>       xsa155-linux-xsa155-0002-xen-netback-don-t-use-last-request-to-determine-mini.patch
>>       xsa155-linux-xsa155-0003-xen-netback-use-RING_COPY_REQUEST-throughout.patch
>>       xsa155-linux-xsa155-0004-xen-blkback-only-read-request-operation-from-shared-.patch
>>       xsa155-linux-xsa155-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>>       xsa155-linux-xsa155-0006-xen-scsiback-safely-copy-requests.patch
>>       xsa155-linux-xsa155-0007-xen-pciback-Save-xen_pci_op-commands-before-processi.patch
>>       Linux v4.[0,1,2,3]
>>       All the above patches except #5 will apply, please use:
>>       xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>>       Linux v3.19:
>>       All the above patches except #5 and #6 will apply, please use:
>>       xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>>       xsa155-linux319-0006-xen-scsiback-safely-copy-requests.patch
>>
>>       qemu-xen:
>>       xsa155-qemu-qdisk-double-access.patch
>>       xsa155-qemu-xenfb.patch
>>
>>       qemu-traditional:
>>       xsa155-qemut-qdisk-double-access.patch
>>       xsa155-qemut-xenfb.patch
>>
>>       NetBSD 7.0:
>>       xsa155-netbsd-xsa155-0001-netbsd-xen-Add-RING_COPY_REQUEST.patch
>>       xsa155-netbsd-xsa155-0002-netbsd-netback-Use-RING_COPY_REQUEST-instead-of-RING.patch
>>       xsa155-netbsd-xsa155-0003-netbsd-ring-Add-barrier-to-provide-an-compiler-barri.patch
>>       xsa155-netbsd-xsa155-0004-netbsd-block-only-read-request-operation-from-shared.patch
>>       xsa155-netbsd-xsa155-0005-netbsd-pciback-Operate-on-local-version-of-xen_pci_o.patch
>>
>>       xen:
>>       xsa155-xen-0001-xen-Add-RING_COPY_REQUEST.patch
>>       xsa155-xen-0002-blktap2-Use-RING_COPY_REQUEST.patch
>>       xsa155-xen-0003-libvchan-Read-prod-cons-only-once.patch
>>
>>       xen 4.4:
>>       All patches except #3 will apply, please use:
>>       xsa155-xen44-0003-libvchan-Read-prod-cons-only-once.patch
>>
>>       $ sha256sum xsa155*
>>       d9fbc104ab2ae797971e351ee0e04e7b7e9c7c33385309bb406c7941dc9a33b4
>>       xsa155-linux319-xsa155-0006-xen-scsiback-safely-copy-requests.patch
>>       590656d83ad7b6052b54659eccb3469658b3942c0dc1366423a66f2f5ac643e1
>>       xsa155-linux43-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>>       2bd18632178e09394c5cd06aded2c14bcc6b6e360ad6e81827d24860fe3e8ca4
>>       xsa155-linux-xsa155-0001-xen-Add-RING_COPY_REQUEST.patch
>>       cecdeccb8e2551252c81fc5f164a8298005df714a574a7ba18b84e8ed5f2bb70
>>       xsa155-linux-xsa155-0002-xen-netback-don-t-use-last-request-to-determine-mini.patch
>>       3916b847243047f0e1053233ade742c14a7f29243584e60bf5db4842a8068855
>>       xsa155-linux-xsa155-0003-xen-netback-use-RING_COPY_REQUEST-throughout.patch
>>       746c8eb0aeb200d76156c88dfbbd49db79f567b88b07eda70f7c7d095721f05a
>>       xsa155-linux-xsa155-0004-xen-blkback-only-read-request-operation-from-shared-.patch
>>       18517a184a02f7441065b8d3423086320ec4c2345c00d551231f7976381767f5
>>       xsa155-linux-xsa155-0005-xen-blkback-read-from-indirect-descriptors-only-once.patch
>>       2e6d556d25b1cc16e71afde665ae3908f4fa8eab7e0d96283fc78400301baf92
>>       xsa155-linux-xsa155-0006-xen-scsiback-safely-copy-requests.patch
>>       5e130d8b61906015c6a94f8edd3cce97b172f96a265d97ecf370e7b45125b73d
>>       xsa155-linux-xsa155-0007-xen-pciback-Save-xen_pci_op-commands-before-processi.patch
>>       08c2d0f95dcc215165afbce623b6972b81dd45b091b5f40017579b00c8612e03
>>       xsa155-netbsd-xsa155-0001-netbsd-xen-Add-RING_COPY_REQUEST.patch
>>       0a66010f736092f91f70bb0fd220685e4395efef1db6d23a3d1eace31d144f51
>>       xsa155-netbsd-xsa155-0002-netbsd-netback-Use-RING_COPY_REQUEST-instead-of-RING.patch
>>       5e913a8427cab6b4d384d1246e05116afc301eb117edd838101eb53a82c2f2ff
>>       xsa155-netbsd-xsa155-0003-netbsd-ring-Add-barrier-to-provide-an-compiler-barri.patch
>>       3b8f14eafaed3a7bc66245753a37af4249acf8129fbedb70653192252dc47dc9
>>       xsa155-netbsd-xsa155-0004-netbsd-block-only-read-request-operation-from-shared.patch
>>       81ae5fa998243a78dad749fc561be647dc1dc1be799e8f18484fdf0989469705
>>       xsa155-netbsd-xsa155-0005-netbsd-pciback-Operate-on-local-version-of-xen_pci_o.patch
>>       044ff74fa048df820d528f64f2791ec9cb3940bd313c1179020bd49a6cde2ca3  xsa155-qemu-qdisk-double-access.patch
>>       1150504589eb7bfa108c80ce63395e57d0e627b12d9201219d968fdd026919a6
>>       xsa155-qemut-qdisk-double-access.patch
>>       63186246ab6913b54bfef5f09f33e815935ac40ff821c27a3efda62339bbbd5f  xsa155-qemut-xenfb.patch
>>       e53b4ac298648cde79344192d5a58ca8d8724344f5105bec7c09eef095c668f6  xsa155-qemu-xenfb.patch
>>       e52467fcec73bcc86d3e96d06f8ca8085ae56a83d2c42a30c16bc3dc630d8f8a
>>       xsa155-xen-0001-xen-Add-RING_COPY_REQUEST.patch
>>       eae34c8ccc096ad93a74190506b3d55020a88afb0cc504a3a514590e9fd746fd
>>       xsa155-xen-0002-blktap2-Use-RING_COPY_REQUEST.patch
>>       42780265014085a4221ad32b026214693d751789eb5219e2e83862c0006c66f4
>>       xsa155-xen-0003-libvchan-Read-prod-cons-only-once.patch
>>       dfcaddb8a908a4fc1b048a43187e885117e67dc566f5c841037ee366dcd437d1
>>       xsa155-xen44-0003-libvchan-Read-prod-cons-only-once.patch
>>       $
>>
>>       DEPLOYMENT DURING EMBARGO
>>       =========================
>>
>>       Deployment of the patches and/or mitigations described above (or
>>       others which are substantially similar) is permitted during the
>>       embargo, even on public-facing systems with untrusted guest users and
>>       administrators.
>>
>>       But: Distribution of updated software is prohibited (except to other
>>       members of the predisclosure list).
>>
>>       Predisclosure list members who wish to deploy significantly different
>>       patches and/or mitigations, please contact the Xen Project Security
>>       Team.
>>
>>       (Note: this during-embargo deployment notice is retained in
>>       post-embargo publicly released Xen Project advisories, even though it
>>       is then no longer applicable.  This is to enable the community to have
>>       oversight of the Xen Project Security Team's decisionmaking.)
>>
>>       For more information about permissible uses of embargoed information,
>>       consult the Xen Project community's agreed Security Policy:
>>         http://www.xenproject.org/security-policy.html
>>       -----BEGIN PGP SIGNATURE-----
>>       Version: GnuPG v1.4.12 (GNU/Linux)
>>
>>       iQEcBAEBAgAGBQJWcrpdAAoJEIP+FMlX6CvZ9soIALqQ/GHP6bZn2LqJTD9DIzsm
>>       zVB4yCPiVfDqHSOq9QNCzBzqpvOX+RhKTzRH1jsZczr8CSnkePxaCrmZgH8SAygB
>>       hFcF9xJGlJDjs647sgpQmYs++3mgD/57uml7IW/8NX46tXUelVByW7muNgUN2xlm
>>       kjeD8auJEs+jK1iwpt/hOmYe4moRx3+3ujfgqMCNAWtqZz9D9wM5tao+p6yKYlhM
>>       u8hSi1V3b7sAbf92mwzpzfpbwdgg25xeHtZ/oJxp/ZY0FhqDEsTxV+h8HjD/Eink
>>       GwqPS19O77tMmz9fUUTyJDSsU7ayFRI0HyYmXju4eJktJkhXagjAdCSyGky9z5g=
>>       =FlX2
>>       -----END PGP SIGNATURE-----
>>
>> ----- Topal: Original message ends -----
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
  2015-12-22 15:06   ` Eric Shelton
@ 2016-01-04 13:06     ` Marek Marczykowski-Górecki
  2016-01-04 15:00       ` Konrad Rzeszutek Wilk
  2016-01-04 16:22       ` David Vrabel
  0 siblings, 2 replies; 9+ messages in thread
From: Marek Marczykowski-Górecki @ 2016-01-04 13:06 UTC (permalink / raw)
  To: Eric Shelton; +Cc: security, xen-devel, Stefano Stabellini


[-- Attachment #1.1.1: Type: text/plain, Size: 1182 bytes --]

On Tue, Dec 22, 2015 at 10:06:25AM -0500, Eric Shelton wrote:
> The XSA mentions that "PV frontend patches will be developed and
> released (publicly) after the embargo date."  Has anything been done
> towards this that should also be incorporated into MiniOS?  On a
> system utilizing a "driver domain," where a backend is running on a
> domain that is considered unprivileged and untrusted (such as the
> example described in http://wiki.xenproject.org/wiki/Driver_Domain),
> it seems XSA-155-style double fetch vulnerabilities in the frontends
> are also a potential security concern, and should be eliminated.
> However, perhaps that does not include pcifront, since pciback would
> always be running in dom0.

And BTW the same applies to Linux frontends, for which also I haven't seen
any public development. In attachment my email to
xen-security-issues-discuss list (sent during embargo), with patches
attached there. I haven't got any response.

PS Dropping minios-devel

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.1.2: Type: message/rfc822, Size: 37548 bytes --]

[-- Attachment #1.1.2.1.1.1.1: Type: text/plain, Size: 4038 bytes --]

On Fri, Dec 11, 2015 at 11:52:17AM -0500, Konrad Rzeszutek Wilk wrote:
> On Fri, Dec 11, 2015 at 12:03:01PM +0100, Marek Marczykowski-Górecki wrote:
> > Hi,
> > 
> > In advisory it's said that PV frontend patches will be developed and
> > released (publicly) after the embargo date. But in some cases (namely:
> > if driver domains are in use), frontends may be more trusted/privileged
> > than backends. So it would make sense to consider developing frontend
> > patches before embargo end date. At least for the most common ones:
> > network and block.
> > 
> > Do you have some list of places that needs to be fixed? I can try to
> > help with patches, but need some input on this...

Patches (against 4.4) for netfront and blkfront attached. I don't have
test case for actual XSA155, but at least it doesn't break normal usage.
Below are some comments/questions.

Sorry for the delay.

> Any code where the shared page is being accessed. For example
> blkif_interrupt.

Added RING_COPY_RESPONSE here. Probably it would be enough to use
just READ_ONCE(bret->operation), but blkif_response isn't that big, so
safer option is better.

The other places in block frontend are about preparing request - it
should not reuse data already put into shared ring, because (malicious)
backend might already changed it. Generally added local structure on the
stack, filled there and only then put in on the ring. And save a copy to
info->shadow[id].req (the original one, not the one from shared ring!).
Maybe it would be better to use info->shadow[id].req in the first place
(and then copy it to the shared ring)? Theoretically one memory copy
less. Does it matter?

> We probably should add another macro: RING_COPY_RESPONSE which
> would replace the RING_GET_RESPONSE and copy the response to
> an on stack variable and then operated on.
> 
> Ditto with xennet_alloc_rx_buffers and the RING_GET_REQUEST.

I'm not sure about this one - there is only write to that request, no
reads. Or maybe I'm missing some obscure code there?

> And
> xennet_tx_buf_gc.

Additionally added BUG_ON(id >= NET_TX_RING_SIZE). Not sure about the
impact of backend giving out of range id, so make sure it's DoS in the
_worst_ case.

> There is also xennet_tx_setup_grant.

It looks like the tx pointer outlive this function. So instead of
changing it to local copy, made sure no data is read from that structure
(only written).

Similar in handling gso in xennet_start_xmit - it's only filled with
data.

Not sure about xennet_move_rx_slot, but I guess it's safe.

> xen-pcifront looks to be safe - it memcopies the fields. However
> as I've found out - doing 'memcpy' does not mean the compiler
> will do that - it may convert that just to a pointer operation.
> So an barrier() will have to be sprinkled in 'do_pci_op'.

There is already wmb() after the first memcpy. Is it necessary to have
another one before the memcpy in the other direction?


$ shasum -a 256 *.patch
f7b0c45dbd2c91bee5cc151b1c53b5b452e4e448991204ab6ac6aeb3b5811674
xsa155-linux-0008-xen-Add-RING_COPY_RESPONSE.patch
a9f34c6b0b746bdd49594ddabab71ff18477b1cbad62d7cd380f3ca5dc9ff7e3
xsa155-linux-0009-xen-netfront-copy-response-out-of-shared-buffer-befo.patch
e924fb7b344135e21c7df9ed6297165275646ea97b9d3072b9f1cc0152d88bec
xsa155-linux-0010-xen-netfront-do-not-use-data-already-exposed-to-back.patch
03ac14dcbfb2ccd9e1c375e57e2c9184075d29ffadbaf696a8b015f7eb6bb2c0
xsa155-linux-0011-xen-netfront-add-range-check-for-Tx-response-id.patch
e71bbd472b33af4424bee936019b11af2dfc1e3512d13d1aad9713fd8b337227
xsa155-linux-0012-xen-blkfront-make-local-copy-of-response-before-usin.patch
4c99b24ed5092e45c504c84312b619cb77c7ee1169b95002a9ea77fb96523d1d
xsa155-linux-0013-xen-blkfront-prepare-request-locally-only-then-put-i.patch

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.1.2.1.1.1.2: xsa155-linux-0008-xen-Add-RING_COPY_RESPONSE.patch --]
[-- Type: text/plain, Size: 2361 bytes --]

From 8322f4eddaf1fe5a9bdf5252c8140daa8bad60fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Tue, 15 Dec 2015 21:35:14 +0100
Subject: [PATCH 08/13] xen: Add RING_COPY_RESPONSE()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Invisible Things Lab
Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Using RING_GET_RESPONSE() on a shared ring is easy to use incorrectly
(i.e., by not considering that the other end may alter the data in the
shared ring while it is being inspected).  Safe usage of a response
generally requires taking a local copy.

Provide a RING_COPY_RESPONSE() macro to use instead of
RING_GET_RESPONSE() and an open-coded memcpy().  This takes care of
ensuring that the copy is done correctly regardless of any possible
compiler optimizations.

Use a volatile source to prevent the compiler from reordering or
omitting the copy.

This is part of XSA155.

CC: stable@vger.kernel.org
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 include/xen/interface/io/ring.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index 7dc685b..312415c 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -198,6 +198,20 @@ struct __name##_back_ring {						\
 #define RING_GET_RESPONSE(_r, _idx)					\
     (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].rsp))
 
+/*
+ * Get a local copy of a response.
+ *
+ * Use this in preference to RING_GET_RESPONSE() so all processing is
+ * done on a local copy that cannot be modified by the other end.
+ *
+ * Note that https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 may cause this
+ * to be ineffective where _rsp is a struct which consists of only bitfields.
+ */
+#define RING_COPY_RESPONSE(_r, _idx, _rsp) do {				\
+	/* Use volatile to force the copy into _rsp. */			\
+	*(_rsp) = *(volatile typeof(_rsp))RING_GET_RESPONSE(_r, _idx);	\
+} while (0)
+
 /* Loop termination condition: Would the specified index overflow the ring? */
 #define RING_REQUEST_CONS_OVERFLOW(_r, _cons)				\
     (((_cons) - (_r)->rsp_prod_pvt) >= RING_SIZE(_r))
-- 
2.1.0


[-- Attachment #1.1.2.1.1.1.3: xsa155-linux-0009-xen-netfront-copy-response-out-of-shared-buffer-befo.patch --]
[-- Type: text/plain, Size: 6318 bytes --]

From 3a1006355114da4b8fc4b935a64928b7f6ae374f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Wed, 16 Dec 2015 05:09:55 +0100
Subject: [PATCH 09/13] xen-netfront: copy response out of shared buffer before
 accessing it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Invisible Things Lab
Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Make local copy of the response, otherwise backend might modify it while
frontend is already processing it - leading to time of check / time of
use issue.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 drivers/net/xen-netfront.c | 51 +++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index d6abf19..2af5100 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -372,13 +372,13 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue)
 		rmb(); /* Ensure we see responses up to 'rp'. */
 
 		for (cons = queue->tx.rsp_cons; cons != prod; cons++) {
-			struct xen_netif_tx_response *txrsp;
+			struct xen_netif_tx_response txrsp;
 
-			txrsp = RING_GET_RESPONSE(&queue->tx, cons);
-			if (txrsp->status == XEN_NETIF_RSP_NULL)
+			RING_COPY_RESPONSE(&queue->tx, cons, &txrsp);
+			if (txrsp.status == XEN_NETIF_RSP_NULL)
 				continue;
 
-			id  = txrsp->id;
+			id  = txrsp.id;
 			skb = queue->tx_skbs[id].skb;
 			if (unlikely(gnttab_query_foreign_access(
 				queue->grant_tx_ref[id]) != 0)) {
@@ -721,7 +721,7 @@ static int xennet_get_extras(struct netfront_queue *queue,
 			     RING_IDX rp)
 
 {
-	struct xen_netif_extra_info *extra;
+	struct xen_netif_extra_info extra;
 	struct device *dev = &queue->info->netdev->dev;
 	RING_IDX cons = queue->rx.rsp_cons;
 	int err = 0;
@@ -737,24 +737,23 @@ static int xennet_get_extras(struct netfront_queue *queue,
 			break;
 		}
 
-		extra = (struct xen_netif_extra_info *)
-			RING_GET_RESPONSE(&queue->rx, ++cons);
+		RING_COPY_RESPONSE(&queue->rx, ++cons, &extra);
 
-		if (unlikely(!extra->type ||
-			     extra->type >= XEN_NETIF_EXTRA_TYPE_MAX)) {
+		if (unlikely(!extra.type ||
+			     extra.type >= XEN_NETIF_EXTRA_TYPE_MAX)) {
 			if (net_ratelimit())
 				dev_warn(dev, "Invalid extra type: %d\n",
-					extra->type);
+					extra.type);
 			err = -EINVAL;
 		} else {
-			memcpy(&extras[extra->type - 1], extra,
-			       sizeof(*extra));
+			memcpy(&extras[extra.type - 1], &extra,
+			       sizeof(extra));
 		}
 
 		skb = xennet_get_rx_skb(queue, cons);
 		ref = xennet_get_rx_ref(queue, cons);
 		xennet_move_rx_slot(queue, skb, ref);
-	} while (extra->flags & XEN_NETIF_EXTRA_FLAG_MORE);
+	} while (extra.flags & XEN_NETIF_EXTRA_FLAG_MORE);
 
 	queue->rx.rsp_cons = cons;
 	return err;
@@ -764,28 +763,28 @@ static int xennet_get_responses(struct netfront_queue *queue,
 				struct netfront_rx_info *rinfo, RING_IDX rp,
 				struct sk_buff_head *list)
 {
-	struct xen_netif_rx_response *rx = &rinfo->rx;
+	struct xen_netif_rx_response rx = rinfo->rx;
 	struct xen_netif_extra_info *extras = rinfo->extras;
 	struct device *dev = &queue->info->netdev->dev;
 	RING_IDX cons = queue->rx.rsp_cons;
 	struct sk_buff *skb = xennet_get_rx_skb(queue, cons);
 	grant_ref_t ref = xennet_get_rx_ref(queue, cons);
-	int max = MAX_SKB_FRAGS + (rx->status <= RX_COPY_THRESHOLD);
+	int max = MAX_SKB_FRAGS + (rx.status <= RX_COPY_THRESHOLD);
 	int slots = 1;
 	int err = 0;
 	unsigned long ret;
 
-	if (rx->flags & XEN_NETRXF_extra_info) {
+	if (rx.flags & XEN_NETRXF_extra_info) {
 		err = xennet_get_extras(queue, extras, rp);
 		cons = queue->rx.rsp_cons;
 	}
 
 	for (;;) {
-		if (unlikely(rx->status < 0 ||
-			     rx->offset + rx->status > XEN_PAGE_SIZE)) {
+		if (unlikely(rx.status < 0 ||
+			     rx.offset + rx.status > XEN_PAGE_SIZE)) {
 			if (net_ratelimit())
 				dev_warn(dev, "rx->offset: %u, size: %d\n",
-					 rx->offset, rx->status);
+					 rx.offset, rx.status);
 			xennet_move_rx_slot(queue, skb, ref);
 			err = -EINVAL;
 			goto next;
@@ -799,7 +798,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
 		if (ref == GRANT_INVALID_REF) {
 			if (net_ratelimit())
 				dev_warn(dev, "Bad rx response id %d.\n",
-					 rx->id);
+					 rx.id);
 			err = -EINVAL;
 			goto next;
 		}
@@ -812,7 +811,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
 		__skb_queue_tail(list, skb);
 
 next:
-		if (!(rx->flags & XEN_NETRXF_more_data))
+		if (!(rx.flags & XEN_NETRXF_more_data))
 			break;
 
 		if (cons + slots == rp) {
@@ -822,7 +821,7 @@ next:
 			break;
 		}
 
-		rx = RING_GET_RESPONSE(&queue->rx, cons + slots);
+		RING_COPY_RESPONSE(&queue->rx, cons + slots, &rx);
 		skb = xennet_get_rx_skb(queue, cons + slots);
 		ref = xennet_get_rx_ref(queue, cons + slots);
 		slots++;
@@ -878,9 +877,9 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue,
 	struct sk_buff *nskb;
 
 	while ((nskb = __skb_dequeue(list))) {
-		struct xen_netif_rx_response *rx =
-			RING_GET_RESPONSE(&queue->rx, ++cons);
+		struct xen_netif_rx_response rx;
 		skb_frag_t *nfrag = &skb_shinfo(nskb)->frags[0];
+		RING_COPY_RESPONSE(&queue->rx, ++cons, &rx);
 
 		if (shinfo->nr_frags == MAX_SKB_FRAGS) {
 			unsigned int pull_to = NETFRONT_SKB_CB(skb)->pull_to;
@@ -891,7 +890,7 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue,
 		BUG_ON(shinfo->nr_frags >= MAX_SKB_FRAGS);
 
 		skb_add_rx_frag(skb, shinfo->nr_frags, skb_frag_page(nfrag),
-				rx->offset, rx->status, PAGE_SIZE);
+				rx.offset, rx.status, PAGE_SIZE);
 
 		skb_shinfo(nskb)->nr_frags = 0;
 		kfree_skb(nskb);
@@ -987,7 +986,7 @@ static int xennet_poll(struct napi_struct *napi, int budget)
 	i = queue->rx.rsp_cons;
 	work_done = 0;
 	while ((i != rp) && (work_done < budget)) {
-		memcpy(rx, RING_GET_RESPONSE(&queue->rx, i), sizeof(*rx));
+		RING_COPY_RESPONSE(&queue->rx, i, rx);
 		memset(extras, 0, sizeof(rinfo.extras));
 
 		err = xennet_get_responses(queue, &rinfo, rp, &tmpq);
-- 
2.1.0


[-- Attachment #1.1.2.1.1.1.4: xsa155-linux-0010-xen-netfront-do-not-use-data-already-exposed-to-back.patch --]
[-- Type: text/plain, Size: 2937 bytes --]

From 2adc557330dde5b474d885518d2663180d3c8f45 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Wed, 16 Dec 2015 05:19:37 +0100
Subject: [PATCH 10/13] xen-netfront: do not use data already exposed to
 backend
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Invisible Things Lab
Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Backend may freely modify anything on shared page, so use data which was
supposed to be written there, instead of reading it back from the shared
page.

This unfortunatelly require putting xennet_make_first_txreq inline into
xennet_start_xmit (the only use), because we need info.size, which isn't
available anywhere else (other than shared page).

This is part of XSA155.

CC: stable@vger.kernel.org
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 drivers/net/xen-netfront.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 2af5100..959e479 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -453,23 +453,7 @@ static void xennet_tx_setup_grant(unsigned long gfn, unsigned int offset,
 	tx->flags = 0;
 
 	info->tx = tx;
-	info->size += tx->size;
-}
-
-static struct xen_netif_tx_request *xennet_make_first_txreq(
-	struct netfront_queue *queue, struct sk_buff *skb,
-	struct page *page, unsigned int offset, unsigned int len)
-{
-	struct xennet_gnttab_make_txreq info = {
-		.queue = queue,
-		.skb = skb,
-		.page = page,
-		.size = 0,
-	};
-
-	gnttab_for_one_grant(page, offset, len, xennet_tx_setup_grant, &info);
-
-	return info.tx;
+	info->size += len;
 }
 
 static void xennet_make_one_txreq(unsigned long gfn, unsigned int offset,
@@ -564,6 +548,7 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct netfront_info *np = netdev_priv(dev);
 	struct netfront_stats *tx_stats = this_cpu_ptr(np->tx_stats);
 	struct xen_netif_tx_request *tx, *first_tx;
+	struct xennet_gnttab_make_txreq info;
 	unsigned int i;
 	int notify;
 	int slots;
@@ -614,14 +599,19 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	/* First request for the linear area. */
-	first_tx = tx = xennet_make_first_txreq(queue, skb,
-						page, offset, len);
-	offset += tx->size;
+	info.queue = queue;
+	info.skb = skb;
+	info.page = page;
+	info.size = 0;
+	gnttab_for_one_grant(page, offset, len, xennet_tx_setup_grant, &info);
+
+	first_tx = tx = info.tx;
+	offset += info.size;
 	if (offset == PAGE_SIZE) {
 		page++;
 		offset = 0;
 	}
-	len -= tx->size;
+	len -= info.size;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL)
 		/* local packet? */
-- 
2.1.0


[-- Attachment #1.1.2.1.1.1.5: xsa155-linux-0011-xen-netfront-add-range-check-for-Tx-response-id.patch --]
[-- Type: text/plain, Size: 1248 bytes --]

From 76a020d3b2023ca02961eab38318ef2d6f1338d9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Wed, 16 Dec 2015 05:22:24 +0100
Subject: [PATCH 11/13] xen-netfront: add range check for Tx response id
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Invisible Things Lab
Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Tx response ID is fetched from shared page, so make sure it is sane
before using it as an array index.

This is part of XSA155.

CC: stable@vger.kernel.org
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 drivers/net/xen-netfront.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 959e479..94309e6 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -379,6 +379,7 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue)
 				continue;
 
 			id  = txrsp.id;
+			BUG_ON(id >= NET_TX_RING_SIZE);
 			skb = queue->tx_skbs[id].skb;
 			if (unlikely(gnttab_query_foreign_access(
 				queue->grant_tx_ref[id]) != 0)) {
-- 
2.1.0


[-- Attachment #1.1.2.1.1.1.6: xsa155-linux-0012-xen-blkfront-make-local-copy-of-response-before-usin.patch --]
[-- Type: text/plain, Size: 4780 bytes --]

From ef0d243bfeaf1da8854c26f89536dc1b69c56602 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Wed, 16 Dec 2015 05:51:10 +0100
Subject: [PATCH 12/13] xen-blkfront: make local copy of response before using
 it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Invisible Things Lab
Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Data on the shared page can be changed at any time by the backend. Make
a local copy, which is no longer controlled by the backend. And only
then access it.

This is part of XSA155.

CC: stable@vger.kernel.org
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 drivers/block/xen-blkfront.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2fee2ee..5d7eb04 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1296,7 +1296,7 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_info *info,
 static irqreturn_t blkif_interrupt(int irq, void *dev_id)
 {
 	struct request *req;
-	struct blkif_response *bret;
+	struct blkif_response bret;
 	RING_IDX i, rp;
 	unsigned long flags;
 	struct blkfront_info *info = (struct blkfront_info *)dev_id;
@@ -1316,8 +1316,8 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
 	for (i = info->ring.rsp_cons; i != rp; i++) {
 		unsigned long id;
 
-		bret = RING_GET_RESPONSE(&info->ring, i);
-		id   = bret->id;
+		RING_COPY_RESPONSE(&info->ring, i, &bret);
+		id   = bret.id;
 		/*
 		 * The backend has messed up and given us an id that we would
 		 * never have given to it (we stamp it up to BLK_RING_SIZE -
@@ -1325,29 +1325,29 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
 		 */
 		if (id >= BLK_RING_SIZE(info)) {
 			WARN(1, "%s: response to %s has incorrect id (%ld)\n",
-			     info->gd->disk_name, op_name(bret->operation), id);
+			     info->gd->disk_name, op_name(bret.operation), id);
 			/* We can't safely get the 'struct request' as
 			 * the id is busted. */
 			continue;
 		}
 		req  = info->shadow[id].request;
 
-		if (bret->operation != BLKIF_OP_DISCARD)
-			blkif_completion(&info->shadow[id], info, bret);
+		if (bret.operation != BLKIF_OP_DISCARD)
+			blkif_completion(&info->shadow[id], info, &bret);
 
 		if (add_id_to_freelist(info, id)) {
 			WARN(1, "%s: response to %s (id %ld) couldn't be recycled!\n",
-			     info->gd->disk_name, op_name(bret->operation), id);
+			     info->gd->disk_name, op_name(bret.operation), id);
 			continue;
 		}
 
-		error = (bret->status == BLKIF_RSP_OKAY) ? 0 : -EIO;
-		switch (bret->operation) {
+		error = (bret.status == BLKIF_RSP_OKAY) ? 0 : -EIO;
+		switch (bret.operation) {
 		case BLKIF_OP_DISCARD:
-			if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
+			if (unlikely(bret.status == BLKIF_RSP_EOPNOTSUPP)) {
 				struct request_queue *rq = info->rq;
 				printk(KERN_WARNING "blkfront: %s: %s op failed\n",
-					   info->gd->disk_name, op_name(bret->operation));
+					   info->gd->disk_name, op_name(bret.operation));
 				error = -EOPNOTSUPP;
 				info->feature_discard = 0;
 				info->feature_secdiscard = 0;
@@ -1358,15 +1358,15 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
 			break;
 		case BLKIF_OP_FLUSH_DISKCACHE:
 		case BLKIF_OP_WRITE_BARRIER:
-			if (unlikely(bret->status == BLKIF_RSP_EOPNOTSUPP)) {
+			if (unlikely(bret.status == BLKIF_RSP_EOPNOTSUPP)) {
 				printk(KERN_WARNING "blkfront: %s: %s op failed\n",
-				       info->gd->disk_name, op_name(bret->operation));
+				       info->gd->disk_name, op_name(bret.operation));
 				error = -EOPNOTSUPP;
 			}
-			if (unlikely(bret->status == BLKIF_RSP_ERROR &&
+			if (unlikely(bret.status == BLKIF_RSP_ERROR &&
 				     info->shadow[id].req.u.rw.nr_segments == 0)) {
 				printk(KERN_WARNING "blkfront: %s: empty %s op failed\n",
-				       info->gd->disk_name, op_name(bret->operation));
+				       info->gd->disk_name, op_name(bret.operation));
 				error = -EOPNOTSUPP;
 			}
 			if (unlikely(error)) {
@@ -1378,9 +1378,9 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
 			/* fall through */
 		case BLKIF_OP_READ:
 		case BLKIF_OP_WRITE:
-			if (unlikely(bret->status != BLKIF_RSP_OKAY))
+			if (unlikely(bret.status != BLKIF_RSP_OKAY))
 				dev_dbg(&info->xbdev->dev, "Bad return from blkdev data "
-					"request: %x\n", bret->status);
+					"request: %x\n", bret.status);
 
 			blk_mq_complete_request(req, error);
 			break;
-- 
2.1.0


[-- Attachment #1.1.2.1.1.1.7: xsa155-linux-0013-xen-blkfront-prepare-request-locally-only-then-put-i.patch --]
[-- Type: text/plain, Size: 6093 bytes --]

From 74aaa42e1f25309a163acd00083ecbbc186fbb47 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Wed, 16 Dec 2015 06:07:14 +0100
Subject: [PATCH 13/13] xen-blkfront: prepare request locally, only then put it
 on the shared ring
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Invisible Things Lab
Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Do not reuse data which theoretically might be already modified by the
backend. This is mostly about private copy of the request
(info->shadow[id].req) - make sure the request saved there is really the
one just filled.

This is part of XSA155.

CC: stable@vger.kernel.org
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 drivers/block/xen-blkfront.c | 56 ++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 5d7eb04..514cf18 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -459,27 +459,29 @@ static int blkif_ioctl(struct block_device *bdev, fmode_t mode,
 static int blkif_queue_discard_req(struct request *req)
 {
 	struct blkfront_info *info = req->rq_disk->private_data;
-	struct blkif_request *ring_req;
+	struct blkif_request ring_req;
 	unsigned long id;
 
 	/* Fill out a communications ring structure. */
-	ring_req = RING_GET_REQUEST(&info->ring, info->ring.req_prod_pvt);
 	id = get_id_from_freelist(info);
 	info->shadow[id].request = req;
 
-	ring_req->operation = BLKIF_OP_DISCARD;
-	ring_req->u.discard.nr_sectors = blk_rq_sectors(req);
-	ring_req->u.discard.id = id;
-	ring_req->u.discard.sector_number = (blkif_sector_t)blk_rq_pos(req);
+	ring_req.operation = BLKIF_OP_DISCARD;
+	ring_req.u.discard.nr_sectors = blk_rq_sectors(req);
+	ring_req.u.discard.id = id;
+	ring_req.u.discard.sector_number = (blkif_sector_t)blk_rq_pos(req);
 	if ((req->cmd_flags & REQ_SECURE) && info->feature_secdiscard)
-		ring_req->u.discard.flag = BLKIF_DISCARD_SECURE;
+		ring_req.u.discard.flag = BLKIF_DISCARD_SECURE;
 	else
-		ring_req->u.discard.flag = 0;
+		ring_req.u.discard.flag = 0;
 
+	/* make the request available to the backend */
+	*RING_GET_REQUEST(&info->ring, info->ring.req_prod_pvt) = ring_req;
+	wmb();
 	info->ring.req_prod_pvt++;
 
 	/* Keep a private copy so we can reissue requests when recovering. */
-	info->shadow[id].req = *ring_req;
+	info->shadow[id].req = ring_req;
 
 	return 0;
 }
@@ -569,7 +571,7 @@ static void blkif_setup_rw_req_grant(unsigned long gfn, unsigned int offset,
 static int blkif_queue_rw_req(struct request *req)
 {
 	struct blkfront_info *info = req->rq_disk->private_data;
-	struct blkif_request *ring_req;
+	struct blkif_request ring_req;
 	unsigned long id;
 	int i;
 	struct setup_rw_req setup = {
@@ -613,7 +615,6 @@ static int blkif_queue_rw_req(struct request *req)
 		new_persistent_gnts = 0;
 
 	/* Fill out a communications ring structure. */
-	ring_req = RING_GET_REQUEST(&info->ring, info->ring.req_prod_pvt);
 	id = get_id_from_freelist(info);
 	info->shadow[id].request = req;
 
@@ -628,7 +629,7 @@ static int blkif_queue_rw_req(struct request *req)
 	for_each_sg(info->shadow[id].sg, sg, num_sg, i)
 	       num_grant += gnttab_count_grant(sg->offset, sg->length);
 
-	ring_req->u.rw.id = id;
+	ring_req.u.rw.id = id;
 	info->shadow[id].num_sg = num_sg;
 	if (num_grant > BLKIF_MAX_SEGMENTS_PER_REQUEST) {
 		/*
@@ -636,16 +637,16 @@ static int blkif_queue_rw_req(struct request *req)
 		 * BLKIF_OP_WRITE
 		 */
 		BUG_ON(req->cmd_flags & (REQ_FLUSH | REQ_FUA));
-		ring_req->operation = BLKIF_OP_INDIRECT;
-		ring_req->u.indirect.indirect_op = rq_data_dir(req) ?
+		ring_req.operation = BLKIF_OP_INDIRECT;
+		ring_req.u.indirect.indirect_op = rq_data_dir(req) ?
 			BLKIF_OP_WRITE : BLKIF_OP_READ;
-		ring_req->u.indirect.sector_number = (blkif_sector_t)blk_rq_pos(req);
-		ring_req->u.indirect.handle = info->handle;
-		ring_req->u.indirect.nr_segments = num_grant;
+		ring_req.u.indirect.sector_number = (blkif_sector_t)blk_rq_pos(req);
+		ring_req.u.indirect.handle = info->handle;
+		ring_req.u.indirect.nr_segments = num_grant;
 	} else {
-		ring_req->u.rw.sector_number = (blkif_sector_t)blk_rq_pos(req);
-		ring_req->u.rw.handle = info->handle;
-		ring_req->operation = rq_data_dir(req) ?
+		ring_req.u.rw.sector_number = (blkif_sector_t)blk_rq_pos(req);
+		ring_req.u.rw.handle = info->handle;
+		ring_req.operation = rq_data_dir(req) ?
 			BLKIF_OP_WRITE : BLKIF_OP_READ;
 		if (req->cmd_flags & (REQ_FLUSH | REQ_FUA)) {
 			/*
@@ -658,21 +659,21 @@ static int blkif_queue_rw_req(struct request *req)
 			switch (info->feature_flush &
 				((REQ_FLUSH|REQ_FUA))) {
 			case REQ_FLUSH|REQ_FUA:
-				ring_req->operation =
+				ring_req.operation =
 					BLKIF_OP_WRITE_BARRIER;
 				break;
 			case REQ_FLUSH:
-				ring_req->operation =
+				ring_req.operation =
 					BLKIF_OP_FLUSH_DISKCACHE;
 				break;
 			default:
-				ring_req->operation = 0;
+				ring_req.operation = 0;
 			}
 		}
-		ring_req->u.rw.nr_segments = num_grant;
+		ring_req.u.rw.nr_segments = num_grant;
 	}
 
-	setup.ring_req = ring_req;
+	setup.ring_req = &ring_req;
 	setup.id = id;
 	for_each_sg(info->shadow[id].sg, sg, num_sg, i) {
 		BUG_ON(sg->offset + sg->length > PAGE_SIZE);
@@ -694,10 +695,13 @@ static int blkif_queue_rw_req(struct request *req)
 	if (setup.segments)
 		kunmap_atomic(setup.segments);
 
+	/* make the request available to the backend */
+	*RING_GET_REQUEST(&info->ring, info->ring.req_prod_pvt) = ring_req;
+	wmb();
 	info->ring.req_prod_pvt++;
 
 	/* Keep a private copy so we can reissue requests when recovering. */
-	info->shadow[id].req = *ring_req;
+	info->shadow[id].req = ring_req;
 
 	if (new_persistent_gnts)
 		gnttab_free_grant_references(setup.gref_head);
-- 
2.1.0


[-- Attachment #1.1.2.1.1.2: Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #1.1.2.1.2: Type: text/plain, Size: 219 bytes --]

_______________________________________________
Xen-security-issues-discuss mailing list
Xen-security-issues-discuss@lists.xenproject.org
http://lists.xenproject.org/cgi-bin/mailman/listinfo/xen-security-issues-discuss

[-- Attachment #1.2: Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
  2016-01-04 13:06     ` Marek Marczykowski-Górecki
@ 2016-01-04 15:00       ` Konrad Rzeszutek Wilk
  2016-01-04 16:22       ` David Vrabel
  1 sibling, 0 replies; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-01-04 15:00 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: security, xen-devel, Stefano Stabellini, Eric Shelton

On Mon, Jan 04, 2016 at 02:06:32PM +0100, Marek Marczykowski-Górecki wrote:
> On Tue, Dec 22, 2015 at 10:06:25AM -0500, Eric Shelton wrote:
> > The XSA mentions that "PV frontend patches will be developed and
> > released (publicly) after the embargo date."  Has anything been done
> > towards this that should also be incorporated into MiniOS?  On a
> > system utilizing a "driver domain," where a backend is running on a
> > domain that is considered unprivileged and untrusted (such as the
> > example described in http://wiki.xenproject.org/wiki/Driver_Domain),
> > it seems XSA-155-style double fetch vulnerabilities in the frontends
> > are also a potential security concern, and should be eliminated.
> > However, perhaps that does not include pcifront, since pciback would
> > always be running in dom0.
> 
> And BTW the same applies to Linux frontends, for which also I haven't seen
> any public development. In attachment my email to
> xen-security-issues-discuss list (sent during embargo), with patches
> attached there. I haven't got any response.

Could you post it using git-send-email please? I took a quick glance at them
but didn't get a chance to do an indepth look.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
  2016-01-04 13:06     ` Marek Marczykowski-Górecki
  2016-01-04 15:00       ` Konrad Rzeszutek Wilk
@ 2016-01-04 16:22       ` David Vrabel
  2016-01-04 16:56         ` Marek Marczykowski-Górecki
  1 sibling, 1 reply; 9+ messages in thread
From: David Vrabel @ 2016-01-04 16:22 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki, Eric Shelton
  Cc: xen-devel, Stefano Stabellini, security

On 04/01/16 13:06, Marek Marczykowski-Górecki wrote:
> On Tue, Dec 22, 2015 at 10:06:25AM -0500, Eric Shelton wrote:
>> The XSA mentions that "PV frontend patches will be developed and
>> released (publicly) after the embargo date."  Has anything been done
>> towards this that should also be incorporated into MiniOS?  On a
>> system utilizing a "driver domain," where a backend is running on a
>> domain that is considered unprivileged and untrusted (such as the
>> example described in http://wiki.xenproject.org/wiki/Driver_Domain),
>> it seems XSA-155-style double fetch vulnerabilities in the frontends
>> are also a potential security concern, and should be eliminated.
>> However, perhaps that does not include pcifront, since pciback would
>> always be running in dom0.
> 
> And BTW the same applies to Linux frontends, for which also I haven't seen
> any public development. In attachment my email to
> xen-security-issues-discuss list (sent during embargo), with patches
> attached there. I haven't got any response.

There are no similar security concerns with frontends since they trust
the backend.

I note that you say:

  "But in some cases (namely: if driver domains are in use), frontends
   may be more trusted/privileged than backends."

But this cannot be the case since the backend can always trivially DoS
the frontend by (for example) not unmapping grant references when
required by the protocol.

David

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
  2016-01-04 16:22       ` David Vrabel
@ 2016-01-04 16:56         ` Marek Marczykowski-Górecki
  2016-01-04 17:37           ` David Vrabel
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Marczykowski-Górecki @ 2016-01-04 16:56 UTC (permalink / raw)
  To: David Vrabel; +Cc: security, xen-devel, Stefano Stabellini, Eric Shelton


[-- Attachment #1.1: Type: text/plain, Size: 1805 bytes --]

On Mon, Jan 04, 2016 at 04:22:32PM +0000, David Vrabel wrote:
> On 04/01/16 13:06, Marek Marczykowski-Górecki wrote:
> > On Tue, Dec 22, 2015 at 10:06:25AM -0500, Eric Shelton wrote:
> >> The XSA mentions that "PV frontend patches will be developed and
> >> released (publicly) after the embargo date."  Has anything been done
> >> towards this that should also be incorporated into MiniOS?  On a
> >> system utilizing a "driver domain," where a backend is running on a
> >> domain that is considered unprivileged and untrusted (such as the
> >> example described in http://wiki.xenproject.org/wiki/Driver_Domain),
> >> it seems XSA-155-style double fetch vulnerabilities in the frontends
> >> are also a potential security concern, and should be eliminated.
> >> However, perhaps that does not include pcifront, since pciback would
> >> always be running in dom0.
> > 
> > And BTW the same applies to Linux frontends, for which also I haven't seen
> > any public development. In attachment my email to
> > xen-security-issues-discuss list (sent during embargo), with patches
> > attached there. I haven't got any response.
> 
> There are no similar security concerns with frontends since they trust
> the backend.
> 
> I note that you say:
> 
>   "But in some cases (namely: if driver domains are in use), frontends
>    may be more trusted/privileged than backends."
> 
> But this cannot be the case since the backend can always trivially DoS
> the frontend by (for example) not unmapping grant references when
> required by the protocol.

DoS is one thing, code execution is another.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory
  2016-01-04 16:56         ` Marek Marczykowski-Górecki
@ 2016-01-04 17:37           ` David Vrabel
  0 siblings, 0 replies; 9+ messages in thread
From: David Vrabel @ 2016-01-04 17:37 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: security, xen-devel, Stefano Stabellini, Eric Shelton

On 04/01/16 16:56, Marek Marczykowski-Górecki wrote:
> On Mon, Jan 04, 2016 at 04:22:32PM +0000, David Vrabel wrote:
>> On 04/01/16 13:06, Marek Marczykowski-Górecki wrote:
>>> On Tue, Dec 22, 2015 at 10:06:25AM -0500, Eric Shelton wrote:
>>>> The XSA mentions that "PV frontend patches will be developed and
>>>> released (publicly) after the embargo date."  Has anything been done
>>>> towards this that should also be incorporated into MiniOS?  On a
>>>> system utilizing a "driver domain," where a backend is running on a
>>>> domain that is considered unprivileged and untrusted (such as the
>>>> example described in http://wiki.xenproject.org/wiki/Driver_Domain),
>>>> it seems XSA-155-style double fetch vulnerabilities in the frontends
>>>> are also a potential security concern, and should be eliminated.
>>>> However, perhaps that does not include pcifront, since pciback would
>>>> always be running in dom0.
>>>
>>> And BTW the same applies to Linux frontends, for which also I haven't seen
>>> any public development. In attachment my email to
>>> xen-security-issues-discuss list (sent during embargo), with patches
>>> attached there. I haven't got any response.
>>
>> There are no similar security concerns with frontends since they trust
>> the backend.
>>
>> I note that you say:
>>
>>   "But in some cases (namely: if driver domains are in use), frontends
>>    may be more trusted/privileged than backends."
>>
>> But this cannot be the case since the backend can always trivially DoS
>> the frontend by (for example) not unmapping grant references when
>> required by the protocol.
> 
> DoS is one thing, code execution is another.

The DoS is a trivial and obvious example to illustrate that your
suggestion that:

"...frontends may be more trusted/privileged than backends."

is ill-advised.

Anyway, none of this means we won't consider your netfront patches.  But
you do need to post them to the correct lists (netdev and xen-devel).

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-01-04 17:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-21 23:10 Xen Security Advisory 155 (CVE-2015-8550) - paravirtualized drivers incautious about shared memory Eric Shelton
2015-12-22 12:24 ` Stefano Stabellini
2015-12-22 13:19   ` Samuel Thibault
2015-12-22 15:06   ` Eric Shelton
2016-01-04 13:06     ` Marek Marczykowski-Górecki
2016-01-04 15:00       ` Konrad Rzeszutek Wilk
2016-01-04 16:22       ` David Vrabel
2016-01-04 16:56         ` Marek Marczykowski-Górecki
2016-01-04 17:37           ` David Vrabel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.