I would restructure it differently. Instead of relying on p_ucred stability under the proc_lock, which is only needed to keep newcred valid, I would do

  newcred = crget();
#ifdef RCTL
  crhold(newcred);
#endif

earlier. Otherwise it depends on the impl details of proc_set_cred(), in particular, that p_ucred is newcred after the call.

Indeed, even if in practice this doesn't make a difference in the current state, relying only on credential references to ensure liveness makes it easier to reason about correctness of this code, and makes potential future changes in this area less error-prone.

This is not the case for this occurrence, but in those of kern_prot.c and in new code in D53457, taking another reference to the new credentials that early (i.e., just after crget()) leads to code "complexification" (e.g., double crfree() in the error paths, which looks quite astonishing).

Since we are relying on credentials references, we still can count on proc_set_cred() taking over the reference to the passed credentials, which in practice means that the additional crhold() can be called only just before calling proc_set_cred(). This makes for a minimal change, so I'd rather implement that variant around your suggestion, even if that means calling crhold() under the process lock (that call is uncontended).

Note that the calls to racct_proc_ucred_changed() are currently also relying on that. In order to suppress this assumption, I had to slightly tweak the order of some operations.