search

Eideticom / eid-hermes

An open-source eBPF accelerator that can run on QEMU and AWS F1 instances
10 stars 2 forks source link

specs: include program length on command request #34

Closed iomartin closed 3 years ago

iomartin commented 3 years ago

An eBPF program can use just part of a program slot and the eBPF engine currently has no way of knowing its length.

This can be a problem if, for example, the eBPF engine tries to validate if a program is valid. For example, in uBPF, the validate() function needs the number of instructions in the program.

A simple solution is to add the program length to the command request.

An alternative would be to modify the XDMA engine to store the program length if it the XDMA target address is in a program slot, but this would be much more complicated.

lsgunth commented 3 years ago

Hmm, I'm a bit worried such a thing could be open for abuse given the user can now give us a different amount of data than they tell us the program is.

Getting this information from the xdma transfer would be much better from this perspective.

It would also be a good idea if the xdma can zero out the rest of the slot every time a transfer occurs.

Security is important and leaving places for the protocol to be abused is not a good idea.

iomartin commented 3 years ago

I agree that this is not ideal, but I feel that we can implement proper checks in the driver and devices to prevent abuse.

Hmm, I'm a bit worried such a thing could be open for abuse given the user can now give us a different amount of data than they tell us the program is.

The program length is already passed by the user when programming the slot via an ioctl and we check that it does not overflow the slot. The driver can just save this length and then use it when it needs to submit the job. Though I'd have to check what happens during the XDMA if the userspace buffer is smaller than the size provided in the ioctl.

Getting this information from the xdma transfer would be much better from this perspective.

This would be challenging, I think. On QEMU I don't think it'd be too bad, but then we'd also have to modify the XDMA RTL and I'd rather we didn't open that can of worms.

It would also be a good idea if the xdma can zero out the rest of the slot every time a transfer occurs.

This is a good idea, I will do this is in another PR.

lsgunth commented 3 years ago

Some things need to be done in the hardware for sensible robustness.... The zeroing should be done in the hardware.... Assuming the driver is sane from the hardware perspective is never a good idea.

Seems like using the XDMA thing has just been a huge barrier to creating a good implementation.

sbates130272 commented 3 years ago

I support this change. I think having the program length in the command makes sense. I agree with @lsgunth that changing the XDMA or moving away from it is preferred but that's just not feasible at this time.

iomartin commented 3 years ago

Getting back to this... Given that hardware changes of this magnitude are not possible at this time, can we merge this? @niclashedam has a qemu prototype that uses this new spec.