--- Begin Message ---
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:
xsa155-linux-0008-xen-Add-RING_COPY_RESPONSE.patch
Description: Text document
Attachment:
xsa155-linux-0009-xen-netfront-copy-response-out-of-shared-buffer-befo.patch
Description: Text document
Attachment:
xsa155-linux-0010-xen-netfront-do-not-use-data-already-exposed-to-back.patch
Description: Text document
Attachment:
xsa155-linux-0011-xen-netfront-add-range-check-for-Tx-response-id.patch
Description: Text document
Attachment:
xsa155-linux-0012-xen-blkfront-make-local-copy-of-response-before-usin.patch
Description: Text document
Attachment:
xsa155-linux-0013-xen-blkfront-prepare-request-locally-only-then-put-i.patch
Description: Text document
Attachment:
pgpvuq3A0QpJR.pgp
Description: PGP signature
_______________________________________________
Xen-security-issues-discuss mailing list
Xen-security-issues-discuss@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/xen-security-issues-discuss
--- End Message ---
