Re: [Patch net v2] cgroup: fix cgroup_sk_alloc() for sk_clone_lock()

[Cong Wang]

On Thu, Jul 9, 2020 at 11:51 AM Cong Wang <xiyou.wangc...@gmail.com> wrote:
>
> On Thu, Jul 9, 2020 at 10:10 AM Guenter Roeck <li...@roeck-us.net> wrote:
> >
> > Something seems fishy with the use of skcd->val on big endian systems.
> >
> > Some debug output:
> >
> > [   22.643703] sock: ##### sk_alloc(sk=000000001be28100): Calling 
> > cgroup_sk_alloc(000000001be28550)
> > [   22.643807] cgroup: ##### cgroup_sk_alloc(skcd=000000001be28550): 
> > cgroup_sk_alloc_disabled=0, in_interrupt: 0
> > [   22.643886] cgroup:  #### cgroup_sk_alloc(skcd=000000001be28550): 
> > cset->dfl_cgrp=0000000001224040, skcd->val=0x1224040
> > [   22.643957] cgroup: ###### cgroup_bpf_get(cgrp=0000000001224040)
> > [   22.646451] sock: ##### sk_prot_free(sk=000000001be28100): Calling 
> > cgroup_sk_free(000000001be28550)
> > [   22.646607] cgroup:  #### sock_cgroup_ptr(skcd=000000001be28550) -> 
> > 0000000000014040 [v=14040, skcd->val=14040]
> > [   22.646632] cgroup: ####### cgroup_sk_free(): skcd=000000001be28550, 
> > cgrp=0000000000014040
> > [   22.646739] cgroup: ####### cgroup_sk_free(): skcd->no_refcnt=0
> > [   22.646814] cgroup: ####### cgroup_sk_free(): Calling 
> > cgroup_bpf_put(cgrp=0000000000014040)
> > [   22.646886] cgroup: ###### cgroup_bpf_put(cgrp=0000000000014040)
>
> Excellent debugging! I thought it was a double put, but it seems to
> be an endian issue. I didn't realize the bit endian machine actually
> packs bitfields in a big endian way too...
>
> Does the attached patch address this?

Ah, this is too ugly. We just have to always make them the last two bits.

Please test this attached patch instead and ignore the previous one.

Thanks.

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 4f1cd0edc57d..0e7a97b50d49 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -792,6 +792,7 @@ struct sock_cgroup_data {
 		struct {
 			u8	is_data : 1;
 			u8	no_refcnt : 1;
+			u8	unused: 6
 			u8	padding;
 			u16	prioidx;
 			u32	classid;
@@ -801,6 +802,7 @@ struct sock_cgroup_data {
 			u32	classid;
 			u16	prioidx;
 			u8	padding;
+			u8	unused: 6
 			u8	no_refcnt : 1;
 			u8	is_data : 1;
 		} __packed;