[tlul] tlul_sram_byte data integrity check

[tjaychen]

When a byte write is issued to memory, tlul_sram_byte is responsible for doing a read first to generate the new word-wise integrity.

However, the read data integrity is not checked as part of this generation. This potentially creates a security risk.

estimate 4

[tjaychen]

@weicaiyang

[weicaiyang]

Let's review this in D2S meeting

[msfschaffner]

@tjaychen, should we just check ECC on all SRAM reads in this case (like we do on incoming TL-UL transactions)?
I.e., if we are adding this logic anyway, we might as let all read transactions benefir from it...

[tjaychen]

o sorry, this is due to lack of a tagging scheme.
This is meant to be "future" improvement. I'm not too concerned about weaknesses in the byte case because we already KNOW it is weaker than a full word write / read.

[msfschaffner]

Ah got it, thanks for the clarification!

FYI @moidx @cfrantz

[moidx]

Tagging with software label as this needs to be added to the software implementation guidelines.

[andreaskurth]

Triaged for tlul. Are we sure we don't want to check the integrity of data read in tlul_sram_byte by adding a tlul_data_integ_dec and feeding its data_err_o into error_o? (This to me does not seem to be a complex change, though we might have to insert some flops to break overly long paths coming out of SRAMs.) Tagging @vogelpi for a security opinion

[vogelpi]

Thanks for tagging me @andreaskurth . After having studied the RTL I agree that the change would probably be rather simple and it would be a nice to have for M2.5. However, it's not critical. Thus, I label this as P3. If we don't have time to do it or things turn out more difficult, we can instead include information on this in the software guidelines.

[msfschaffner]

Given that byte-writes are already considered less secure due to the ECC recomputation, I would lump this into software guidance for now and forego the RTL change.

[msfschaffner]

@moidx since this is documentation-only, I'll move this to the M2.5 Backlog.

[msfschaffner]

@vogelpi @johannheyszl are we ok with leaving this in the documentation-only category for Earlgrey-PROD?
If so, does it make sense to capture that in the SW guidelines?

[johannheyszl]

Both ways - adding to guidelines, or a modification - sgtm. If the fix is small and we can avoid one more item on the guidelines, that is a benefit. @vogelpi ?

[msfschaffner]

I would lean towards no change at this point, since this also introduces again more DV effort.

[vogelpi]

Ideally, we would have changed the design here but I agree to leave this as is for Earlgrey-PROD. There are other, higher priority things to be taken care of first.

[vogelpi]

This was discussed in the Sec WG meeting on Mar 14, 2024 . It was concluded to leave this as is.

There are also other reasons for not doing byte writes and we have already SW guidance for those. For example, byte-write operations are not favorable from an SCA viewpoint. In both cases, SW should first perform a word read and then insert the byte inside the ALU and write back the full word.