On Thu, Apr 18, 2019 at 10:42 PM Yang Yingliang <yangyingliang@xxxxxxxxxx> wrote: > On 2019/4/19 10:04, Paul Moore wrote: > > On Wed, Apr 17, 2019 at 10:50 PM Yang Yingliang > > <yangyingliang@xxxxxxxxxx> wrote: > >> On 2019/4/18 8:24, Casey Schaufler wrote: > >>> On 4/17/2019 4:39 PM, Paul Moore wrote: > >>>> Since it looks like all three LSMs which implement the setprocattr > >>>> hook are vulnerable I'm open to the idea that proc_pid_attr_write() is > >>>> a better choice for the cred != read_cred check, but I would want to > >>>> make sure John and Casey are okay with that. > >>>> > >>>> John? > >>>> > >>>> Casey? > >>> I'm fine with the change going into proc_pid_attr_write(). > >> The cred != real_cred checking is not enough. > >> > >> Consider this situation, when doing override, cred, real_cred and > >> new_cred are all same: > >> > >> after override_creds() cred == real_cred == new1_cred > > I'm sorry, you've lost me. After override_creds() returns > > current->cred and current->real_cred are not going to be the same, > > yes? > > It's possible the new cred is equal to current->real_cred and > current->cred, > so after overrides_creds(), they have the same value. Both task_struct.cred and task_struct.real_cred are pointer values, assuming that one uses prepare_creds() to allocate/initialize a new cred struct for use with override_creds() then the newly created cred should never be equal to task_struct.real_cred. Am I missing something, or are you thinking of something else? -- paul moore www.paul-moore.com
