On 10/09/26 10:40:04, Jorge Ramirez wrote:
> On 10/09/26 10:55:47, Ilias Apalodimas wrote:
> > Hi Jorge,
> >
> > [...]
> >
> > > +
> > > +void optee_suppl_cmd_rpmb_probe_next(struct udevice *dev,
> > > + struct optee_msg_arg *arg)
> > > +{
> > > + struct optee_private *priv = dev_get_priv(dev);
> > > + struct udevice *scsi_dev;
> > > + struct tee_shm *cid_shm;
> > > + u8 size_mult = 0;
> > > + u8 rel_wr = 0;
> > > + void *cid_buf;
> > > + ulong cid_size;
> > > + int ret;
> > > +
> > > + if (arg->num_params != 2 ||
> > > + arg->params[0].attr != OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT ||
> > > + arg->params[1].attr != OPTEE_MSG_ATTR_TYPE_RMEM_OUTPUT) {
> > > + arg->ret = TEE_ERROR_BAD_PARAMETERS;
> > > + return;
> > > + }
> > > +
> > > + cid_shm = (struct tee_shm *)(ulong)arg->params[1].u.rmem.shm_ref;
> > > + cid_buf = (u8 *)cid_shm->addr + arg->params[1].u.rmem.offs;
> > > + cid_size = arg->params[1].u.rmem.size;
> > > + if (cid_size < UFS_RPMB_CID_SIZE) {
> > > + arg->ret = TEE_ERROR_SHORT_BUFFER;
> > > + return;
> > > + }
> > > +
> > > + if (optee_rpmb_get_dev(&scsi_dev)) {
> > > + arg->ret = TEE_ERROR_ITEM_NOT_FOUND;
> > > + return;
> > > + }
> > > +
> > > + while (priv->rpmb_next_region < UFS_RPMB_NUM_REGIONS) {
> > > + unsigned int region = priv->rpmb_next_region++;
> > > +
> >
> > We had a brief chat offline, but here's what I think is happening.
> >
> > OP-TEE and U-Boot have their own view of 'partitions to write'.
> > What I think is happening here if the UFS RPMB partitions is > 1 is
> > - For OP-TEE have_cand is only set once, for the first partition and
> > restored just before writing
> > the key
> > - U-Boot sets rpmb_cur_region independantly and exits whenever it exhausts
> > all the RPMB partitions
> > - OP-TEE will restore the first key and probably write it to the wrong slot
> >
> > I haven't checked OP-TEE enough, but since this is an OTP I think we either
> > need a way to verify we
> > are writing the right key to the right slot, or the partition number
> > actually calculated and
> > instructed by OP-TEE (in the TA arguments perhaps?)
> >
> > I really prefer the latter, but I understand it's a much bigger change,
> > plus the OP-TEE code is
> > already merged. So there's always the chance that someone is unlucky enough
> > to checkout a wrong
> > combination of U-Boot/OP-TEE and 'brick' his RPMB.
> >
> > It apparently didn't show up on any testing, because it works fine for a
> > single partition.
>
>
> thanks Ilias - you are 100% right.
>
> A last minute change went in op-tee to scan all available RPMBs
> partitions and restore a context which wouldnt move the u-boot
> index.
>
> we can address it in op-tee via CFG_ (simple, the config already exists,
> we just apply the write instead of postponing it); and in parallel
> develop a more user friendly approach as the one you described. I think
> we all agree are better solution is required.
>
https://github.com/OP-TEE/optee_os/pull/8007