[linux-morello] Re: [PATCH v4 1/1] fs: Handle exact bounds for argv and envp

On 14/11/2023 12:06, Kevin Brodsky wrote:

On 13/11/2023 19:51, Teo Couprie Diaz wrote:

...

[...]

• /*

• * The start of the string should always be properly aligned, but
  

• * its representable length might be different. Get the representable
  

• * length by using the same length that was used during allocation:
  

• * the length of the original string.
  

• * This takes into account the padding due to length change, but not that
  

• * for alignment. Thus we might not end up at the start of the next arg.
  

• * If not, we will need to take a slow
  

• * path to go through the padding.
  

Nit: the text could be rewrapped (we end up with two short lines at the end).

...

• */
  

• len = cheri_representable_length(len);

+#if (ELF_COMPAT == 0)

• str_ptr = cheri_perms_and(str_ptr,

• (CHERI_PERM_GLOBAL | CHERI_PERM_STORE | CHERI_PERM_LOAD));
  

• str_ptr = cheri_bounds_set_exact(str_ptr, len);

+#endif

• if (elf_stack_put_user(str_ptr, stack_item++))

• return -EFAULT;
  

• /*

• * If right after the end of the argument length we have a zero,
  

• * that means the argument alignment was adjusted in order to create a
  

• * representable capability in purecap, even if we are not loading a
  

• * purecap binary. This padding is added at the end, so find the real
  

• * end by going through the padding.
  

• */
  

• for (pad_len = 0; len + pad_len < MAX_ARG_STRLEN; pad_len++) {

• if (get_user(c, ustr + len + pad_len))
  

• 	return -EFAULT;
  

• if (c != '\0')
  

• 	break;
  

• }
• ustr += pad_len;
• len += pad_len;

+#else /* CONFIG_CHERI_PURECAP_UABI */

• len = strnlen_user(ustr, MAX_ARG_STRLEN);
• if (!len || len > MAX_ARG_STRLEN)

• return -EINVAL;
  

I somehow missed this in earlier versions but these 3 lines are the same in all cases, so we could move them out of #ifdef / #else.

Indeed, very much so ! Missed it as well.

Nothing else to complain about, the patch looks very good now, well done! I can make those small tweaks myself before merging if that's fine with you.

Kevin

Happy for you to do those small tweaks !

Thanks a lot for all your comments, I'm very happy with the state of the patch and to have wrapped up this work.