Memory bounds checking

[mndstrmr]

An earlier issue I described was about an overflow in bounds checking for memory operations. Logic to fix this has been put in place, but does not do so.

Specifically, top_vio = (top_chkaddr > top_bound) && top_size_ok; should be top_vio = (top_chkaddr > top_bound) || ~top_size_ok;

Also the top_size_ok never checks rf_fullcap_a.top33[32], despite that being acceptable. (This presumably prevents root capabilities from being used in memory operations)

[kliuMsft]

You are right - will fix. My bad for too much double-negativity.

[kliuMsft]

Fixed in commit ba458ab. Please verify. Thanks,

[mndstrmr]

In cheri_ex.sv, addr_bound_vio should still be set for top_equal in the is_cap case (i.e. a CLC for address a when top = a should fail):

if (debug_mode_i)
  addr_bound_vio = 1'b0;
else if (cheri_operator_i[CIS_SUBSET])  // Need to double check this, but this sounds right
  addr_bound_vio = top_vio | base_vio;
else if (is_cap)
  addr_bound_vio = top_vio | base_vio | top_equal;
else if (cheri_operator_i[CJAL] | cheri_operator_i[CJALR])
  ...

Otherwise, would appear to be proving.

[kliuMsft]

In the CLC/CSC case, we have

base_chkaddr = cs1_addr_plusimm; top_chkaddr = {base_chkaddr[31:3], 3'b000}; top_bound = {rf_fullcap_a.top33[32:3], 3'b000};

And we are checking for address alignment perm_vio_vec[PVIO_ALIGN] = (cs1_addr_plusimm[2:0] != 0);

So it seems to be top_equal is actually not a bound violation, as we are comparing the rounded down (to 8-byte boundary) version of top33 vs the also rounded-down version of address. I agree the math is a little convoluted. Setting top_check_addr = {base_chkaddr[31:3], 3'b111} might be the better way to go, in which case top_equal is indeed a violation.

From: Louis-Emile Ploix @.> Sent: Saturday, January 20, 2024 8:32 AM To: microsoft/cheriot-ibex @.> Cc: Comment @.>; Subscribed @.> Subject: Re: [microsoft/cheriot-ibex] Memory bounds checking (Issue #12)

In cheri_ex.sv, addr_bound_vio should still be set for top_equal in the is_cap case (i.e. a CLC for address a when top = a should fail):

if (debug_mode_i)

addr_bound_vio = 1'b0;

else if (cheri_operator_i[CIS_SUBSET]) // Need to double check this, but this sounds right

addr_bound_vio = top_vio | base_vio;

else if (is_cap)

addr_bound_vio = top_vio | base_vio | top_equal;

else if (cheri_operator_i[CJAL] | cheri_operator_i[CJALR])

...

Otherwise, would appear to be proving.

- Reply to this email directly, view it on GitHubhttps://github.com/microsoft/cheriot-ibex/issues/12#issuecomment-1902169039 or unsubscribehttps://github.com/notifications/unsubscribe-auth/A3V7IMFOPMN5GXZ5ISBOY7LYPPWRFBFKMF2HI4TJMJ2XIZLTSOBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLLDTOVRGUZLDORPXI6LQMWWES43TOVSUG33NNVSW45FGORXXA2LDOOJIFJDUPFYGLKTSMVYG643JORXXE6NFOZQWY5LFVE2TSMBRGM3DKOBZQKSHI6LQMWSWS43TOVS2K5TBNR2WLKRRHEYTGNRRGE3DGM5HORZGSZ3HMVZKMY3SMVQXIZI. 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.

[kliuMsft]

Never mind - you are right, top_equal is indeed a violation for clc/csc cases. I got confused last night by the double negatives. Will check in the fix soon.

[kliuMsft]

Checked in the fix (commit [5c37f9a]). Please verify, thanks.

[mndstrmr]

Fix is correct, but I did suggest the wrong fix: See #11