CSetBounds does not check that addr >= base

[mndstrmr]

I brought this problem up earlier in an email, but it was suggested that it was an artefact of allowing illegal capabilities to be formed. This was plausible at the time, as I was indeed allowing unrealistic capabilities to be loaded from memory. This is fixed now, yet the issue persists without using memory operations at all.

I believe that CSetBounds is currently allowing the lower bound to be reduced below the current lower bound. In particular I believe that at the below location in set_bounds a check for addr < in_cap.base32 is required: https://github.com/microsoft/cheriot-ibex/blob/a3ce0260ee0d7bbde9ef334fb91ddf3fe6f694ee/rtl/cheri_pkg.sv#L449

A comment just above suggests that addr >= base, but I don't see why that should be true: This is not checked, and it is easy to create capabilities where that is not the case. The above fix does prove.

[kliuMsft]

@rmn30. Very interesting.. What I had in mind is that if the input (CS1) cap is valid, then we should always have cs1.addr >= cs1.base (which also implies cd.base >= cs1.base, since cd.exp <= cs1.exp). This is based on the assumption that we always do representability check when set address, so in any case cap.addr < cap.base will result in tag clearing. But maybe I am missing something here? Could you come up with an counter example..

[rmn30]

The encoding can only represent caps with addr >= base so that must be true of cs1 input (there may be some unused encodings with addr < base but they should never have tag set). I assume out_cap.valid is initially set to in_cap.valid?

[mndstrmr]

I do not see why addr >= base should be maintained, there is nothing as far as I can tell in the logic of set_address to prevent setting an address below the base, and counter examples can be easily found disagreeing with this which just immediately use cincaddr. Consider the following counter example (apologies for the somewhat arbitrary numbers):

cspecialr r0, mtdc
cincaddr r0, r0, #0x74000010 (constant is actually in a reg, but how its loaded is irrelevant)
csetbounds r0, r0, #0x74000010 (same here)
cincaddrimm r0, r0, #-0x7d1 (This moves the address below the base. Nothing prevents this, tag is still set)
csetbounds r0, r0, #0x74000010 (same with constant again, and again tag is still set)

[kliuMsft]

Very good point - thanks for the counter-example.

@rmn30, what I am thinking is that in this case, the problem is with setaddr (4th step), which should clear the tag since the new addr is indeed below the base. Basically our current representability check doesn't handle the case where exp=24. Actually it looks like sail doesn't clear tag in this case either, so was there a particular reason for it?

[kliuMsft]

Also, @rmn30, I recall some earlier discussions about in the case of exp=24, it's actually always representable, which I do agree. However wouldn't it be better if we make it consistent with other exp cases..

Another point is that if we do add a check of (addr >= new_base) in this case (csetbounds), it does put an extra comparator in the critical timing path. If we only have to do (addr >= old_base), it's not as much a problem.

[kliuMsft]

@mndstrmr , for now I checked in a RTL update (ad90fc7) to add the check to clear tag when (addr > old_base).

[mndstrmr]

Fix proves correct, thanks.

[kliuMsft]

Great, thanks!

From: Louis-Emile Ploix @.> Sent: Monday, February 5, 2024 8:28 AM To: microsoft/cheriot-ibex @.> Cc: Comment @.>; Subscribed @.> Subject: Re: [microsoft/cheriot-ibex] CSetBounds does not check that addr >= base (Issue #21)

Closed #21https://github.com/microsoft/cheriot-ibex/issues/21 as completed.

- Reply to this email directly, view it on GitHubhttps://github.com/microsoft/cheriot-ibex/issues/21 or unsubscribehttps://github.com/notifications/unsubscribe-auth/A3V7IMAJAIVWVLM2RWNQIULYSEB75BFKMF2HI4TJMJ2XIZLTSOBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJLJONZXKZNENZQW2ZNLORUHEZLBMRPXI6LQMWBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTLDTOVRGUZLDORPXI6LQMWSUS43TOVS2M5DPOBUWG44SQKSHI6LQMWVHEZLQN5ZWS5DPOJ42K5TBNR2WLKJVHEYDCMZWGU4DTAVEOR4XAZNFNFZXG5LFUV3GC3DVMWVDEMJQGM3DKMBTGY2KO5DSNFTWOZLSUZRWY33TMVSA. You are receiving this email because you commented on the thread.

Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.