netdev - Re: [PATCHv3, ipsec-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 30 Jan 2015 10:11:42 +0800
From:	Fan Du <fengyuleidian0615@...il.com>
To:	nicolas.dichtel@...nd.com
CC:	Fan Du <fan.du@...el.com>, steffen.klassert@...unet.com,
	herbert@...dor.apana.org.au, davem@...emloft.net,
	netdev@...r.kernel.org
Subject: Re: [PATCHv3, ipsec-next] xfrm: Do not parse 32bits compiled xfrm
 netlink msg on 64bits host

于 2015年01月29日 18:29, Nicolas Dichtel 写道:
> Le 27/01/2015 10:00, Fan Du a écrit :
>> structure like xfrm_usersa_info or xfrm_userpolicy_info
>> has different sizeof when compiled as 32bits and 64bits
>> due to not appending pack attribute in their definition.
>> This will result in broken SA and SP information when user
>> trying to configure them through netlink interface.
>>
>> Inform user land about this situation instead of keeping
>> silent, the upper test scripts would behave accordingly.
>>
>> Quotes from: http://marc.info/?l=linux-netdev&m=142226348715503&w=2
>>>
>>> Before a clean solution show up, I think it's better to warn user in some way
>>> like http://patchwork.ozlabs.org/patch/323842/ did. Otherwise, many people
>>> who stuck there will always spend time and try to fix this issue in whatever way.
>>
>> Yes, this is the first thing we should do. I'm willing to accept a patch
>>
>> Signed-off-by: Fan Du <fan.du@...el.com>
> A way to solve this problem was to provide to userland a xfrm compat header
> file, which match the ABI of the kernel. Something like:
>
> #include <linux/xfrm.h>
>
> #define xfrm_usersa_info xfrm_usersa_info_64
> #define xfrm_usersa_info_compat xfrm_usersa_info
> struct xfrm_usersa_info_compat {
>      struct xfrm_selector        sel;
>      struct xfrm_id            id;
>      xfrm_address_t            saddr;
>      struct xfrm_lifetime_cfg    lft;
>      struct xfrm_lifetime_cur    curlft;
>      struct xfrm_stats        stats;
>      __u32                seq;
>      __u32                reqid;
>      __u16                family;
>      __u8                mode;
>      __u8                replay_window;
>      __u8                flags;
>      __u8                hole1;
>      __u32                hole2;
> };
>
> The point I try to make is that patching userland apps allows to use xfrm on a
> 32bits userland / 64bits kernel.
>
> If I understand well your patch, it will not be possible anymore, all messages
> will be rejected. And this may break existing apps.

Add padding in user space does not fix existing 32bits ip binary, moreover not sure all other ARCHes
require the same padding. Maillist has various of proposals AFAICT, all is rejected for reasons
whatever for a very long time including padding user space structure. In fact those structures
are exposed by uapi from kernel to userspace.

Speaking of "break", run 32bits ip xfrm {state/policy/monitor} is _already_ broken on 64bits host.
This patch is making user not stumble there by seeing invalid xfrm information and wondering what's
going on. They can switch to setkey temporally.

last, thanks for your comments after looking at the code...

>
> Regards,
> Nicolas

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html