qemu-ppc
[Top][All Lists]
Advanced

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

Re: [Qemu-ppc] [PATCH RESEND v2 2/3] sPAPR: Support RTAS call ibm, {open


From: Alexey Kardashevskiy
Subject: Re: [Qemu-ppc] [PATCH RESEND v2 2/3] sPAPR: Support RTAS call ibm, {open, close}-errinjct
Date: Tue, 4 Aug 2015 17:23:30 +1000
User-agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

On 08/04/2015 05:16 PM, Gavin Shan wrote:
On Tue, Aug 04, 2015 at 02:49:14PM +1000, Alexey Kardashevskiy wrote:
On 08/03/2015 01:32 PM, Gavin Shan wrote:
On Mon, Aug 03, 2015 at 12:51:09PM +1000, David Gibson wrote:
On Mon, Aug 03, 2015 at 09:23:19AM +1000, Gavin Shan wrote:
The patch supports RTAS calls "ibm,{open,close}-errinjct" to
manupliate the token, which is passed to RTAS call "ibm,errinjct"
to indicate the valid context for error injection. Each VM is
permitted to have only one token at once and we simply have one
random number for that.

Signed-off-by: Gavin Shan <address@hidden>
---
  hw/ppc/spapr_rtas.c    | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++
  include/hw/ppc/spapr.h |  9 ++++++-
  2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index e99e25f..0a9c904 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -604,6 +604,73 @@ out:
      rtas_st(rets, 0, rc);
  }

+static void rtas_ibm_open_errinjct(PowerPCCPU *cpu,
+                                   sPAPRMachineState *spapr,
+                                   uint32_t token, uint32_t nargs,
+                                   target_ulong args, uint32_t nret,
+                                   target_ulong rets)
+{
+    int32_t ret;
+
+    /* Sanity check on number of arguments */
+    if ((nargs != 0) || (nret != 2)) {
+        ret = RTAS_OUT_PARAM_ERROR;
+        goto out;
+    }
+
+    /* Check if we already had token */
+    if (spapr->errinjct_token) {
+        ret = RTAS_OUT_TOKEN_OPENED;
+        goto out;
+    }
+
+    /* Grab random number as token */
+    spapr->errinjct_token = random();

I don't quite understand the function of this token.   Using random()
seems a very, very odd way of doing things.  Is it supposed to be a
security thing?


Yes, the token is allocated by "ibm,open-errinjct". The token will be
passed to subsequent "ibm,errinjct" and "ibm,close-errinjct". From this
perspecitve, the token owner is allowed to do error injection and it's
for security. Apart from having random number as the token, is there
better (fast) way to produce it?

+    if (spapr->errinjct_token == 0) {
+        ret = RTAS_OUT_BUSY;

AFAICT, this gives a 1 in RAND_MAX chance of returning RTAS_OUT_BUSY
for no particular reason.


Yes, "0" represents invalid token (not opened). Maybe here we can retry
for a bit more like below. 0 returned from 10 successive random() would
be rare.

     uint32_t retries;

     while (!spapr->errinjct_token && retries++ < 10)
         spapr->errinjct_token = random();
     if (!spapr->errinjct_token) {
         ret = RTAS_OUT_BUSY;
         goto out;
     }


No. QEMU is using rand() (not random()) and since it returns up to RAND_MAX
which is 0x7fffffff, you could do something simple like this:

spapr->errinjct_token = (rand % 32767) + 1


Good idea. I'll have it in next revision.

Thanks,
Gavin


But for debugging purposes it makes more sense just to initialize it to 1 and
then increment it in every call of rtas_ibm_open_errinjct().


Why rand() and not this? You do not protect against a guest attack by limiting a number of the rtas calls so the token just needs to be unique and that's it, and later in gdb is is going to be easier to trace these tokens if need for this ever arises.



--
Alexey



reply via email to

[Prev in Thread] Current Thread [Next in Thread]