| Message ID | 2a32c7c2048333169c9378194d6a435e2e7ed2d7.camel@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: optee: allow plain TMEM buffers with NULL address | expand |
(+Paul) Hi, On 18/05/2020 02:53, Volodymyr Babchuk wrote: > Trusted Applications use popular approach to determine required size > of buffer: client provides a memory reference with the NULL pointer to > a buffer. This is so called "Null memory reference". TA updates the NIT: You use double space after '.' here but all the others use a single space. > reference with the required size and returns it back to client. Then > client allocates buffer of needed size and repeats the operation. > > This behavior is described in TEE Client API Specification, paragraph > 3.2.5. Memory References. From the spec, it is not a clear cut that NULL will always used as fetching the required size of an output buffer. In particular, they suggest to refer to the protocol. In your commit message you indirectly point to an example where 0/NULL would have a different meaning depending on the flags. This is not explained in the TEE Client API Specification. Do you have some pointer I could use to check the behavior? > > OP-TEE represents this null memory reference as a TMEM parameter with > buf_ptr == NULL. This is the only case when we should allow TMEM > buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer at address 0" but with the flag cleared, it would mean "return the size". Am I correct? > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> The code looks to match your commit message, but I wasn't able to match it with the spec. Do you have other pointer I could use to check the behavior? I assume this wants to be part of Xen 4.14. The change is only for OP-TEE which is a tech preview feature. So the risk is very limited. Cheers,
Hi Paul, Julien, Volodymyr, This is another bug fix that I would like to get in 4.14. It doesn't look like we need any changes to this patch, assuming that it is accurate to the spec. It would be nice if Volodymyr could provide more info on the spec side, but honestly I trust his experience on this. Paul, are you OK with this going into 4.14? On Sat, 6 Jun 2020, Julien Grall wrote: > (+Paul) > > Hi, > > On 18/05/2020 02:53, Volodymyr Babchuk wrote: > > Trusted Applications use popular approach to determine required size > > of buffer: client provides a memory reference with the NULL pointer to > > a buffer. This is so called "Null memory reference". TA updates the > > NIT: You use double space after '.' here but all the others use a single > space. > > > reference with the required size and returns it back to client. Then > > client allocates buffer of needed size and repeats the operation. > > > > This behavior is described in TEE Client API Specification, paragraph > > 3.2.5. Memory References. > > From the spec, it is not a clear cut that NULL will always used as fetching > the required size of an output buffer. In particular, they suggest to refer to > the protocol. > > In your commit message you indirectly point to an example where 0/NULL would > have a different meaning depending on the flags. This is not explained in the > TEE Client API Specification. Do you have some pointer I could use to check > the behavior? > > > > > OP-TEE represents this null memory reference as a TMEM parameter with > > buf_ptr == NULL. This is the only case when we should allow TMEM > > buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. > > IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer at > address 0" but with the flag cleared, it would mean "return the size". Am I > correct? > > > > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > The code looks to match your commit message, but I wasn't able to match it > with the spec. Do you have other pointer I could use to check the behavior? > > I assume this wants to be part of Xen 4.14. The change is only for OP-TEE > which is a tech preview feature. So the risk is very limited.
Hi Julien, Julien Grall writes: > (+Paul) > > Hi, > > On 18/05/2020 02:53, Volodymyr Babchuk wrote: >> Trusted Applications use popular approach to determine required size >> of buffer: client provides a memory reference with the NULL pointer to >> a buffer. This is so called "Null memory reference". TA updates the > > NIT: You use double space after '.' here but all the others use a > single space. Oops, missed that. >> reference with the required size and returns it back to client. Then >> client allocates buffer of needed size and repeats the operation. >> >> This behavior is described in TEE Client API Specification, paragraph >> 3.2.5. Memory References. > > From the spec, it is not a clear cut that NULL will always used as > fetching the required size of an output buffer. In particular, they > suggest to refer to the protocol. > > In your commit message you indirectly point to an example where 0/NULL > would have a different meaning depending on the flags. This is not > explained in the TEE Client API Specification. Do you have some > pointer I could use to check the behavior? Well, GP specification describes application interface. It does not specifies how implementation should handle this internally. Basically, GP defines functions, data types, macros, etc to be used by CAs and TAs. But it does not define how exactly call from CA will be delivered to TA. Implementation is free to use any means as far, as they conform with rules described in the specification. OPTEE's REE<->TEE interface is described in the header files, that I added to xen (optee_msg.h, optee_rpc_cmd.h,optee_smc.h). But it does not describe every aspect, unfortunately. >> >> OP-TEE represents this null memory reference as a TMEM parameter with >> buf_ptr == NULL. This is the only case when we should allow TMEM >> buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. > > IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer > at address 0" but with the flag cleared, it would mean "return the > size". Am I correct? Not exactly. This flag does not affect behavior for buffers with address NULL. In any case, this would not mean "return the size" to OP-TEE. This is because OP-TEE works as a transport between CA and TA and it does not assign any meaning to client buffers. But certainly, null buffer will mean "return the size" for some TAs, as described in GlobalPlatform specification. Reason why I prohibited buffers without OPTEE_MSG_ATTR_NONCONTIG flag in the the original patch is that such buffers could span across page boundary, which is no-go for fragmented p2m. But I missed that special case: null buffer without OPTEE_MSG_ATTR_NONCONTIG flag. As this is a special type of buffer, it can be allowed with and without said flag. > >> >> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > The code looks to match your commit message, but I wasn't able to > match it with the spec. Do you have other pointer I could use to check > the behavior? > > I assume this wants to be part of Xen 4.14. The change is only for > OP-TEE which is a tech preview feature. So the risk is very limited. Sure.
Hi Stefano, Stefano Stabellini writes: > Hi Paul, Julien, Volodymyr, > > This is another bug fix that I would like to get in 4.14. It doesn't > look like we need any changes to this patch, assuming that it is > accurate to the spec. > > It would be nice if Volodymyr could provide more info on the spec side, > but honestly I trust his experience on this. I'm sorry for the delay. Julien asks very good and deep questions and it sometimes requires deep research, to fully and correctly answer to them. This was exactly that case. I'll answer timely to the next e-mails.
> -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Sent: 18 June 2020 23:27 > To: xadimgnik@gmail.com; paul@xen.org; pdurrant@amazon.com > Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; sstabellini@kernel.org; xen- > devel@lists.xenproject.org; paul@xen.org; julien@xen.org > Subject: Re: [PATCH] xen/arm: optee: allow plain TMEM buffers with NULL address > > Hi Paul, Julien, Volodymyr, > > This is another bug fix that I would like to get in 4.14. It doesn't > look like we need any changes to this patch, assuming that it is > accurate to the spec. > > It would be nice if Volodymyr could provide more info on the spec side, > but honestly I trust his experience on this. > > Paul, are you OK with this going into 4.14? > In principle, yes. It appears, from Julien's comments though, that the commit message may need a little expansion (for the benefit of those mining this code in future). Paul > > > On Sat, 6 Jun 2020, Julien Grall wrote: > > (+Paul) > > > > Hi, > > > > On 18/05/2020 02:53, Volodymyr Babchuk wrote: > > > Trusted Applications use popular approach to determine required size > > > of buffer: client provides a memory reference with the NULL pointer to > > > a buffer. This is so called "Null memory reference". TA updates the > > > > NIT: You use double space after '.' here but all the others use a single > > space. > > > > > reference with the required size and returns it back to client. Then > > > client allocates buffer of needed size and repeats the operation. > > > > > > This behavior is described in TEE Client API Specification, paragraph > > > 3.2.5. Memory References. > > > > From the spec, it is not a clear cut that NULL will always used as fetching > > the required size of an output buffer. In particular, they suggest to refer to > > the protocol. > > > > In your commit message you indirectly point to an example where 0/NULL would > > have a different meaning depending on the flags. This is not explained in the > > TEE Client API Specification. Do you have some pointer I could use to check > > the behavior? > > > > > > > > OP-TEE represents this null memory reference as a TMEM parameter with > > > buf_ptr == NULL. This is the only case when we should allow TMEM > > > buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. > > > > IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer at > > address 0" but with the flag cleared, it would mean "return the size". Am I > > correct? > > > > > > > > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > > The code looks to match your commit message, but I wasn't able to match it > > with the spec. Do you have other pointer I could use to check the behavior? > > > > I assume this wants to be part of Xen 4.14. The change is only for OP-TEE > > which is a tech preview feature. So the risk is very limited.
On 19/06/2020 00:29, Volodymyr Babchuk wrote: > > Hi Julien, Hi Volodymyr, > > Julien Grall writes: > >> (+Paul) >> >> Hi, >> >> On 18/05/2020 02:53, Volodymyr Babchuk wrote: >>> Trusted Applications use popular approach to determine required size >>> of buffer: client provides a memory reference with the NULL pointer to >>> a buffer. This is so called "Null memory reference". TA updates the >> >> NIT: You use double space after '.' here but all the others use a >> single space. > > Oops, missed that. > >>> reference with the required size and returns it back to client. Then >>> client allocates buffer of needed size and repeats the operation. >>> >>> This behavior is described in TEE Client API Specification, paragraph >>> 3.2.5. Memory References. >> >> From the spec, it is not a clear cut that NULL will always used as >> fetching the required size of an output buffer. In particular, they >> suggest to refer to the protocol. >> >> In your commit message you indirectly point to an example where 0/NULL >> would have a different meaning depending on the flags. This is not >> explained in the TEE Client API Specification. Do you have some >> pointer I could use to check the behavior? > > Well, GP specification describes application interface. It does not > specifies how implementation should handle this internally. Basically, > GP defines functions, data types, macros, etc to be used by CAs and > TAs. But it does not define how exactly call from CA will be delivered > to TA. Implementation is free to use any means as far, as they conform > with rules described in the specification. > > OPTEE's REE<->TEE interface is described in the header files, that I > added to xen (optee_msg.h, optee_rpc_cmd.h,optee_smc.h). But it does not > describe every aspect, unfortunately. Thanks for digging-through! More below. > >>> >>> OP-TEE represents this null memory reference as a TMEM parameter with >>> buf_ptr == NULL. This is the only case when we should allow TMEM >>> buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. >> >> IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer >> at address 0" but with the flag cleared, it would mean "return the >> size". Am I correct? > > Not exactly. This flag does not affect behavior for buffers with address > NULL. In any case, this would not mean "return the size" to > OP-TEE. This is because OP-TEE works as a transport between CA and TA > and it does not assign any meaning to client buffers. But certainly, > null buffer will mean "return the size" for some TAs, as described in > GlobalPlatform specification. Does it mean a guest TA may potentially access address 0? I am asking that because 0 can be a valid host physical address. We are currently carving out 0 from the heap allocator to workaround a bug, but there is no promise this address will be used by the boot allocator and therefore Xen. > Reason why I prohibited buffers without OPTEE_MSG_ATTR_NONCONTIG flag in > the the original patch is that such buffers could span across page > boundary, which is no-go for fragmented p2m. > > But I missed that special case: null buffer without > OPTEE_MSG_ATTR_NONCONTIG flag. As this is a special type of buffer, it > can be allowed with and without said flag. Looking at translate_noncontig(), there is no specific treatment for NULL. So the address will get translated. This is likely to result to an error as usually a guest doesn't have anything mapped at address 0. Did I miss anything? Cheers,
Julien Grall writes: > On 19/06/2020 00:29, Volodymyr Babchuk wrote: >> >> Hi Julien, > > Hi Volodymyr, > >> >> Julien Grall writes: >> >>> (+Paul) >>> >>> Hi, >>> >>> On 18/05/2020 02:53, Volodymyr Babchuk wrote: >>>> Trusted Applications use popular approach to determine required size >>>> of buffer: client provides a memory reference with the NULL pointer to >>>> a buffer. This is so called "Null memory reference". TA updates the >>> >>> NIT: You use double space after '.' here but all the others use a >>> single space. >> >> Oops, missed that. >> >>>> reference with the required size and returns it back to client. Then >>>> client allocates buffer of needed size and repeats the operation. >>>> >>>> This behavior is described in TEE Client API Specification, paragraph >>>> 3.2.5. Memory References. >>> >>> From the spec, it is not a clear cut that NULL will always used as >>> fetching the required size of an output buffer. In particular, they >>> suggest to refer to the protocol. >>> >>> In your commit message you indirectly point to an example where 0/NULL >>> would have a different meaning depending on the flags. This is not >>> explained in the TEE Client API Specification. Do you have some >>> pointer I could use to check the behavior? >> >> Well, GP specification describes application interface. It does not >> specifies how implementation should handle this internally. Basically, >> GP defines functions, data types, macros, etc to be used by CAs and >> TAs. But it does not define how exactly call from CA will be delivered >> to TA. Implementation is free to use any means as far, as they conform >> with rules described in the specification. >> >> OPTEE's REE<->TEE interface is described in the header files, that I >> added to xen (optee_msg.h, optee_rpc_cmd.h,optee_smc.h). But it does not >> describe every aspect, unfortunately. > > Thanks for digging-through! More below. > >> >>>> >>>> OP-TEE represents this null memory reference as a TMEM parameter with >>>> buf_ptr == NULL. This is the only case when we should allow TMEM >>>> buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. >>> >>> IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer >>> at address 0" but with the flag cleared, it would mean "return the >>> size". Am I correct? >> >> Not exactly. This flag does not affect behavior for buffers with address >> NULL. In any case, this would not mean "return the size" to >> OP-TEE. This is because OP-TEE works as a transport between CA and TA >> and it does not assign any meaning to client buffers. But certainly, >> null buffer will mean "return the size" for some TAs, as described in >> GlobalPlatform specification. > > Does it mean a guest TA may potentially access address 0? TA is running in S-EL0. That buffer with PA=0x0 will be not mapped in TA address space at all. So, if TA will try to access address 0, it will be terminated with data abort. > I am asking that because 0 can be a valid host physical address. We > are currently carving out 0 from the heap allocator to workaround a > bug, but there is no promise this address will be used by the boot > allocator and therefore Xen. > Well, this is a potential issue in OP-TEE. It does not treat 0 as a valid address. So, in that rare case, when REE will try to use 0 as a correct address for data buffer, OP-TEE will not map it into TA address space, instead it will pass NULL to TA, so TA will think that no buffer was provided. >> Reason why I prohibited buffers without OPTEE_MSG_ATTR_NONCONTIG flag in >> the the original patch is that such buffers could span across page >> boundary, which is no-go for fragmented p2m. >> >> But I missed that special case: null buffer without >> OPTEE_MSG_ATTR_NONCONTIG flag. As this is a special type of buffer, it >> can be allowed with and without said flag. > > Looking at translate_noncontig(), there is no specific treatment for > NULL. So the address will get translated. This is likely to result to > an error as usually a guest doesn't have anything mapped at address 0. Yes, you are right. Right now, optee client driver will not emit buffer wit OPTEE_MSG_ATTR_NONCONTIG and address 0. But this is possible. And this should be handled correctly. I'll add fix for that. Today, as well. Thanks for pointing this out.
Hi Volodymyr, On 19/06/2020 10:52, Volodymyr Babchuk wrote: >>>>> OP-TEE represents this null memory reference as a TMEM parameter with >>>>> buf_ptr == NULL. This is the only case when we should allow TMEM >>>>> buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. >>>> >>>> IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer >>>> at address 0" but with the flag cleared, it would mean "return the >>>> size". Am I correct? >>> >>> Not exactly. This flag does not affect behavior for buffers with address >>> NULL. In any case, this would not mean "return the size" to >>> OP-TEE. This is because OP-TEE works as a transport between CA and TA >>> and it does not assign any meaning to client buffers. But certainly, >>> null buffer will mean "return the size" for some TAs, as described in >>> GlobalPlatform specification. >> >> Does it mean a guest TA may potentially access address 0? > > TA is running in S-EL0. That buffer with PA=0x0 will be not mapped in TA > address space at all. So, if TA will try to access address 0, it > will be terminated with data abort. > >> I am asking that because 0 can be a valid host physical address. We >> are currently carving out 0 from the heap allocator to workaround a >> bug, but there is no promise this address will be used by the boot >> allocator and therefore Xen. >> > > Well, this is a potential issue in OP-TEE. It does not treat 0 as a > valid address. So, in that rare case, when REE will try to use 0 > as a correct address for data buffer, OP-TEE will not map it into > TA address space, instead it will pass NULL to TA, so TA will think that > no buffer was provided. Thanks! That's reassuring. Although, I think we may still need to prevent MFN 0 to be allocated for a guest using OP-TEE. So they don't end up with weird failure. I don't think this is an issue so far, but this may change with Stefano's dom0less series to support direct mapping. Cheers,
On Fri, 19 Jun 2020, Julien Grall wrote: > Hi Volodymyr, > > On 19/06/2020 10:52, Volodymyr Babchuk wrote: > > > > > > OP-TEE represents this null memory reference as a TMEM parameter > > > > > > with > > > > > > buf_ptr == NULL. This is the only case when we should allow TMEM > > > > > > buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. > > > > > > > > > > IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer > > > > > at address 0" but with the flag cleared, it would mean "return the > > > > > size". Am I correct? > > > > > > > > Not exactly. This flag does not affect behavior for buffers with address > > > > NULL. In any case, this would not mean "return the size" to > > > > OP-TEE. This is because OP-TEE works as a transport between CA and TA > > > > and it does not assign any meaning to client buffers. But certainly, > > > > null buffer will mean "return the size" for some TAs, as described in > > > > GlobalPlatform specification. > > > > > > Does it mean a guest TA may potentially access address 0? > > > > TA is running in S-EL0. That buffer with PA=0x0 will be not mapped in TA > > address space at all. So, if TA will try to access address 0, it > > will be terminated with data abort. > > > > > I am asking that because 0 can be a valid host physical address. We > > > are currently carving out 0 from the heap allocator to workaround a > > > bug, but there is no promise this address will be used by the boot > > > allocator and therefore Xen. > > > > > > > Well, this is a potential issue in OP-TEE. It does not treat 0 as a > > valid address. So, in that rare case, when REE will try to use 0 > > as a correct address for data buffer, OP-TEE will not map it into > > TA address space, instead it will pass NULL to TA, so TA will think that > > no buffer was provided. > > Thanks! That's reassuring. Although, I think we may still need to prevent MFN > 0 to be allocated for a guest using OP-TEE. So they don't end up with weird > failure. > > I don't think this is an issue so far, but this may change with Stefano's > dom0less series to support direct mapping. Yes, it is interesting to know about this limitation. In regards to this patch, it looks OK as-is in terms of code changes. Aside from a link to this conversation in the xen-devel archives, is there anything else you would like to add to the commit message?
On 19/06/2020 18:15, Stefano Stabellini wrote: > On Fri, 19 Jun 2020, Julien Grall wrote: >> Hi Volodymyr, >> >> On 19/06/2020 10:52, Volodymyr Babchuk wrote: >>>>>>> OP-TEE represents this null memory reference as a TMEM parameter >>>>>>> with >>>>>>> buf_ptr == NULL. This is the only case when we should allow TMEM >>>>>>> buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. >>>>>> >>>>>> IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the buffer >>>>>> at address 0" but with the flag cleared, it would mean "return the >>>>>> size". Am I correct? >>>>> >>>>> Not exactly. This flag does not affect behavior for buffers with address >>>>> NULL. In any case, this would not mean "return the size" to >>>>> OP-TEE. This is because OP-TEE works as a transport between CA and TA >>>>> and it does not assign any meaning to client buffers. But certainly, >>>>> null buffer will mean "return the size" for some TAs, as described in >>>>> GlobalPlatform specification. >>>> >>>> Does it mean a guest TA may potentially access address 0? >>> >>> TA is running in S-EL0. That buffer with PA=0x0 will be not mapped in TA >>> address space at all. So, if TA will try to access address 0, it >>> will be terminated with data abort. >>> >>>> I am asking that because 0 can be a valid host physical address. We >>>> are currently carving out 0 from the heap allocator to workaround a >>>> bug, but there is no promise this address will be used by the boot >>>> allocator and therefore Xen. >>>> >>> >>> Well, this is a potential issue in OP-TEE. It does not treat 0 as a >>> valid address. So, in that rare case, when REE will try to use 0 >>> as a correct address for data buffer, OP-TEE will not map it into >>> TA address space, instead it will pass NULL to TA, so TA will think that >>> no buffer was provided. >> >> Thanks! That's reassuring. Although, I think we may still need to prevent MFN >> 0 to be allocated for a guest using OP-TEE. So they don't end up with weird >> failure. >> >> I don't think this is an issue so far, but this may change with Stefano's >> dom0less series to support direct mapping. > > Yes, it is interesting to know about this limitation. > > In regards to this patch, it looks OK as-is in terms of code changes. I would disagree here. NULL has never been handled correctly for TMEM buffers (see [1]). I would argue this needs to be folded within this patch rather than be a separate one. > Aside from a link to this conversation in the xen-devel archives, is > there anything else you would like to add to the commit message? +1 for the link. However, I don't think the commit message fully match/summarize the discussion here. What needs to be clearly spell out is that existing OP-TEEs will never map MFN 0. Cheers, [1] <87h7v71ixf.fsf@epam.com>
On Fri, 19 Jun 2020, Julien Grall wrote: > On 19/06/2020 18:15, Stefano Stabellini wrote: > > On Fri, 19 Jun 2020, Julien Grall wrote: > > > Hi Volodymyr, > > > > > > On 19/06/2020 10:52, Volodymyr Babchuk wrote: > > > > > > > > OP-TEE represents this null memory reference as a TMEM parameter > > > > > > > > with > > > > > > > > buf_ptr == NULL. This is the only case when we should allow TMEM > > > > > > > > buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. > > > > > > > > > > > > > > IIUC, 0 with OPTEE_MSG_ATTR_NONCONTIG set would mean "use the > > > > > > > buffer > > > > > > > at address 0" but with the flag cleared, it would mean "return the > > > > > > > size". Am I correct? > > > > > > > > > > > > Not exactly. This flag does not affect behavior for buffers with > > > > > > address > > > > > > NULL. In any case, this would not mean "return the size" to > > > > > > OP-TEE. This is because OP-TEE works as a transport between CA and > > > > > > TA > > > > > > and it does not assign any meaning to client buffers. But certainly, > > > > > > null buffer will mean "return the size" for some TAs, as described > > > > > > in > > > > > > GlobalPlatform specification. > > > > > > > > > > Does it mean a guest TA may potentially access address 0? > > > > > > > > TA is running in S-EL0. That buffer with PA=0x0 will be not mapped in TA > > > > address space at all. So, if TA will try to access address 0, it > > > > will be terminated with data abort. > > > > > > > > > I am asking that because 0 can be a valid host physical address. We > > > > > are currently carving out 0 from the heap allocator to workaround a > > > > > bug, but there is no promise this address will be used by the boot > > > > > allocator and therefore Xen. > > > > > > > > > > > > > Well, this is a potential issue in OP-TEE. It does not treat 0 as a > > > > valid address. So, in that rare case, when REE will try to use 0 > > > > as a correct address for data buffer, OP-TEE will not map it into > > > > TA address space, instead it will pass NULL to TA, so TA will think that > > > > no buffer was provided. > > > > > > Thanks! That's reassuring. Although, I think we may still need to prevent > > > MFN > > > 0 to be allocated for a guest using OP-TEE. So they don't end up with > > > weird > > > failure. > > > > > > I don't think this is an issue so far, but this may change with Stefano's > > > dom0less series to support direct mapping. > > > > Yes, it is interesting to know about this limitation. > > > > In regards to this patch, it looks OK as-is in terms of code changes. > > I would disagree here. NULL has never been handled correctly for TMEM buffers > (see [1]). I would argue this needs to be folded within this patch rather than > be a separate one. I am OK with that too. > > Aside from a link to this conversation in the xen-devel archives, is > > there anything else you would like to add to the commit message? > +1 for the link. However, I don't think the commit message fully > match/summarize the discussion here. > > What needs to be clearly spell out is that existing OP-TEEs will never map MFN > 0. That would be nice > Cheers, > > [1] <87h7v71ixf.fsf@epam.com>
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c index af19fc31f8..fb7d491b25 100644 --- a/xen/arch/arm/tee/optee.c +++ b/xen/arch/arm/tee/optee.c @@ -865,9 +865,12 @@ static int translate_params(struct optee_domain *ctx, } else { - gdprintk(XENLOG_WARNING, "Guest tries to use old tmem arg\n"); - ret = -EINVAL; - goto out; + if ( call->xen_arg->params[i].u.tmem.buf_ptr ) + { + gdprintk(XENLOG_WARNING, "Guest tries to use old tmem arg\n"); + ret = -EINVAL; + goto out; + }
Trusted Applications use popular approach to determine required size of buffer: client provides a memory reference with the NULL pointer to a buffer. This is so called "Null memory reference". TA updates the reference with the required size and returns it back to client. Then client allocates buffer of needed size and repeats the operation. This behavior is described in TEE Client API Specification, paragraph 3.2.5. Memory References. OP-TEE represents this null memory reference as a TMEM parameter with buf_ptr == NULL. This is the only case when we should allow TMEM buffer without the OPTEE_MSG_ATTR_NONCONTIG flag. Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- xen/arch/arm/tee/optee.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) } break; case OPTEE_MSG_ATTR_TYPE_NONE: