2020-04-02 06:03:02

by Amol Grover

[permalink] [raw]
Subject: [PATCH 1/3 RESEND] sched: Remove __rcu annotation from cred pointer

task_struct::cred (subjective credentials) is *always* used
task-synchronously, hence, does not require RCU semantics.

task_struct::real_cred (objective credentials) can be used in
RCU context and its __rcu annotation is retained.

However, task_struct::cred and task_struct::real_cred *may*
point to the same object, hence, the object pointed to by
task_struct::cred *may* have RCU delayed freeing.

Suggested-by: Jann Horn <[email protected]>
Co-developed-by: Joel Fernandes (Google) <[email protected]>
Signed-off-by: Joel Fernandes (Google) <[email protected]>
Signed-off-by: Amol Grover <[email protected]>
---
include/linux/sched.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 716ad1d8d95e..39924e6e0cf2 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -879,8 +879,11 @@ struct task_struct {
/* Objective and real subjective task credentials (COW): */
const struct cred __rcu *real_cred;

- /* Effective (overridable) subjective task credentials (COW): */
- const struct cred __rcu *cred;
+ /*
+ * Effective (overridable) subjective task credentials (COW)
+ * which is used task-synchronously
+ */
+ const struct cred *cred;

#ifdef CONFIG_KEYS
/* Cached requested key. */
--
2.24.1


2020-05-24 08:18:23

by Amol Grover

[permalink] [raw]
Subject: Re: [PATCH 1/3 RESEND] sched: Remove __rcu annotation from cred pointer

On Thu, Apr 02, 2020 at 11:26:38AM +0530, Amol Grover wrote:
> task_struct::cred (subjective credentials) is *always* used
> task-synchronously, hence, does not require RCU semantics.
>
> task_struct::real_cred (objective credentials) can be used in
> RCU context and its __rcu annotation is retained.
>
> However, task_struct::cred and task_struct::real_cred *may*
> point to the same object, hence, the object pointed to by
> task_struct::cred *may* have RCU delayed freeing.
>
> Suggested-by: Jann Horn <[email protected]>
> Co-developed-by: Joel Fernandes (Google) <[email protected]>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> Signed-off-by: Amol Grover <[email protected]>

Hello everyone,

Could you please go through patches 1/3 and 2/3 and if deemed OK, give
your acks. I sent the original patch in beginning of February (~4 months
back) and resent the patches again in beginning of April due to lack of
traffic. Paul Moore was kind enough to ack twice - the 3/3 and its
resend patch. However these 2 patches still remain. I'd really
appreciate if someone reviewed them.

Thanks
Amol

2020-05-25 13:22:32

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH 1/3 RESEND] sched: Remove __rcu annotation from cred pointer

On 2020-05-24 13:41, Amol Grover wrote:
> On Thu, Apr 02, 2020 at 11:26:38AM +0530, Amol Grover wrote:
> > task_struct::cred (subjective credentials) is *always* used
> > task-synchronously, hence, does not require RCU semantics.
> >
> > task_struct::real_cred (objective credentials) can be used in
> > RCU context and its __rcu annotation is retained.
> >
> > However, task_struct::cred and task_struct::real_cred *may*
> > point to the same object, hence, the object pointed to by
> > task_struct::cred *may* have RCU delayed freeing.
> >
> > Suggested-by: Jann Horn <[email protected]>
> > Co-developed-by: Joel Fernandes (Google) <[email protected]>
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > Signed-off-by: Amol Grover <[email protected]>
>
> Hello everyone,
>
> Could you please go through patches 1/3 and 2/3 and if deemed OK, give
> your acks. I sent the original patch in beginning of February (~4 months
> back) and resent the patches again in beginning of April due to lack of
> traffic. Paul Moore was kind enough to ack twice - the 3/3 and its
> resend patch. However these 2 patches still remain. I'd really
> appreciate if someone reviewed them.

I asked on April 3 which upstream tree you expect this patchset to go
through and I did not see a reply. Do you have a specific target or is
the large addressee list assuming someone else is taking this set? All
we have seen is that it is not intended to go through the audit tree.

> Thanks
> Amol

- RGB

--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

2020-05-25 22:18:06

by Amol Grover

[permalink] [raw]
Subject: Re: [PATCH 1/3 RESEND] sched: Remove __rcu annotation from cred pointer

On Mon, May 25, 2020 at 09:17:41AM -0400, Richard Guy Briggs wrote:
> On 2020-05-24 13:41, Amol Grover wrote:
> > On Thu, Apr 02, 2020 at 11:26:38AM +0530, Amol Grover wrote:
> > > task_struct::cred (subjective credentials) is *always* used
> > > task-synchronously, hence, does not require RCU semantics.
> > >
> > > task_struct::real_cred (objective credentials) can be used in
> > > RCU context and its __rcu annotation is retained.
> > >
> > > However, task_struct::cred and task_struct::real_cred *may*
> > > point to the same object, hence, the object pointed to by
> > > task_struct::cred *may* have RCU delayed freeing.
> > >
> > > Suggested-by: Jann Horn <[email protected]>
> > > Co-developed-by: Joel Fernandes (Google) <[email protected]>
> > > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > > Signed-off-by: Amol Grover <[email protected]>
> >
> > Hello everyone,
> >
> > Could you please go through patches 1/3 and 2/3 and if deemed OK, give
> > your acks. I sent the original patch in beginning of February (~4 months
> > back) and resent the patches again in beginning of April due to lack of
> > traffic. Paul Moore was kind enough to ack twice - the 3/3 and its
> > resend patch. However these 2 patches still remain. I'd really
> > appreciate if someone reviewed them.
>
> I asked on April 3 which upstream tree you expect this patchset to go
> through and I did not see a reply. Do you have a specific target or is
> the large addressee list assuming someone else is taking this set? All
> we have seen is that it is not intended to go through the audit tree.
>

Apologies for it. As Paul Moore replied, initially I assumed this
patchset to not go through the audit tree as the audit specific changes
were secondary to the main change (though certainly I did not think
which upstream tree the patchset would go through). But now I am okay
with the patchset making it to upstream via audit tree if it is fine by
the maintainers.

Thanks
Amol

> > Thanks
> > Amol
>
> - RGB
>
> --
> Richard Guy Briggs <[email protected]>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635
>

2020-05-26 13:09:26

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 1/3 RESEND] sched: Remove __rcu annotation from cred pointer

On Mon, May 25, 2020 at 2:04 PM Amol Grover <[email protected]> wrote:
> On Mon, May 25, 2020 at 09:17:41AM -0400, Richard Guy Briggs wrote:
> > On 2020-05-24 13:41, Amol Grover wrote:
> > > On Thu, Apr 02, 2020 at 11:26:38AM +0530, Amol Grover wrote:
> > > > task_struct::cred (subjective credentials) is *always* used
> > > > task-synchronously, hence, does not require RCU semantics.
> > > >
> > > > task_struct::real_cred (objective credentials) can be used in
> > > > RCU context and its __rcu annotation is retained.
> > > >
> > > > However, task_struct::cred and task_struct::real_cred *may*
> > > > point to the same object, hence, the object pointed to by
> > > > task_struct::cred *may* have RCU delayed freeing.
> > > >
> > > > Suggested-by: Jann Horn <[email protected]>
> > > > Co-developed-by: Joel Fernandes (Google) <[email protected]>
> > > > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > > > Signed-off-by: Amol Grover <[email protected]>
> > >
> > > Hello everyone,
> > >
> > > Could you please go through patches 1/3 and 2/3 and if deemed OK, give
> > > your acks. I sent the original patch in beginning of February (~4 months
> > > back) and resent the patches again in beginning of April due to lack of
> > > traffic. Paul Moore was kind enough to ack twice - the 3/3 and its
> > > resend patch. However these 2 patches still remain. I'd really
> > > appreciate if someone reviewed them.
> >
> > I asked on April 3 which upstream tree you expect this patchset to go
> > through and I did not see a reply. Do you have a specific target or is
> > the large addressee list assuming someone else is taking this set? All
> > we have seen is that it is not intended to go through the audit tree.
> >
>
> Apologies for it. As Paul Moore replied, initially I assumed this
> patchset to not go through the audit tree as the audit specific changes
> were secondary to the main change (though certainly I did not think
> which upstream tree the patchset would go through). But now I am okay
> with the patchset making it to upstream via audit tree if it is fine by
> the maintainers.

This patchset is not appropriate for the audit tree as the most
significant changes are not audit related.

My ACK on patch 3/3 was, and is, conditional on the previous patches
being acceptable to the greater kernel community; this is the main
reason why I didn't ACK patch 1/3 or 2/3.

--
paul moore
http://www.paul-moore.com