Re: [PATCH net-next] {selinux, af_key} Rework pfkey_sadb2xfrm_user_sec_ctx

[Paul Moore <paul@paul-moore.com>]

On Thursday, October 17, 2013 11:51:48 AM Steffen Klassert wrote:
> On Thu, Oct 17, 2013 at 09:34:53AM +0800, Fan Du wrote:
> > On 2013年10月16日 23:15, Paul Moore wrote:
> > >The fact that you are now changing sadb_x_sec_ctx->sadb_x_sec_len
> > >whenever
> > >pfkey_sadb2xfrm_user_sec_ctx() is called raises an eyebrow.  Can you
> > >elaborate on why this is not a problem?
> > 
> > Thanks for your attention, Paul.
> > 
> > sadb_x_sec_ctx is extra headers passed down from user space, the usage of
> > of this data structure falls down to one of pfkey_funcs function only for
> > one time, more specifically speaking, it's only used by SELINUX for
> > security checking for each operation. In other words, sadb_x_sec_ctx
> > involves with a one shot business here. So the original codes seems do a
> > lots of extra job which could easily be avoid using casting operation.
> 
> Since the selinux people have to live with that change in the fist place,
> I'd like to see an ack of one of the selinux maintainers before I take
> in into ipsec-next, Paul?

Well, my earlier concern over modifying the length field probably isn't a 
major concern as was pointed out so I could maybe look the other way on that 
point.  However, while looking a bit closer at the structs and how they are 
used, I noticed that PFKEY structs are all explicitly packed (sadb_x_sec_ctx) 
and the LSM struct is not (xfrm_user_sec_ctx).  I'm not enough of a compiler 
guru across all the different architectures to know if this is significant or 
not given the structure, but it does make me pause.

Any compiler gurus care to weigh in on this?

-- 
paul moore
www.paul-moore.com